#1014 文字間隔に 1以上の値を設定すると改行がうまく扱えなくなる

[ds14050]

一般掲示板[8087]と サクラエディタふぁんくらぶ part17の 632の件です。それぞれの内容は以下の通り。

(1)タイプ別設定で、「文字の間隔」に１以上を設定する。
(2)行末文字「$」を（正規表現にチェック）して「置換」する。
　 →余分な改行が1つ追加される。
(3)行末文字「＄」を（正規表現にチェック）して「すべて置換」する。
　 →余分な改行が50ぐらい追加される。
(4)複数行を選択して行末文字「＄」を（正規表現にチェック）して「すべて置換」する。>　 →余分な改行が無限ループで追加される続ける。

2.3.0.0
下記の設定の場合、行頭の改行が削除できない
改行コードが、LF
文字の間隔が、1ドット

原因は LogicToLayout と LayoutToLogic の非対称かと思います。

CLayoutMgr::LogicToLayoutは改行文字の幅として 1 だけ加算します。
http://sourceforge.net/p/sakura-editor/code/4035/tree/sakura/trunk2/sakura_core/doc/layout/CLayoutMgr.cpp#l934
※ちなみにこの代入の前後で nCaretPos の値は文字間隔分だけ減少します。というのは、ループの最後のイテレーションで改行は 文字間隔+1 の幅を与えられて nCaretPosX に組み入れられているからです。

一方、CLayoutMgr::LayoutToLogicEx は改行文字(※改行は常にCLogicInt(1)の長さです)を含むループ
http://sourceforge.net/p/sakura-editor/code/4035/tree/sakura/trunk2/sakura_core/doc/layout/CLayoutMgr.cpp#l1030
の最後のイテレーション(i == nDataLen-1)において LogicToLayoutのときと同様に
nCharKetas = GetLayoutXOfChar( pData, nDataLen, i );
によって改行に 文字間隔+1 の幅を与えます。これによりループの終了条件
if( nX + nCharKetas > ptLayout.GetX2() && !bEOF ){
break;
}
を早期に満たしてしまうため、文字間隔が 0より大きい場合に、最後の非改行文字+1 のレイアウト座標を与えられたときでも改行文字をポイントすることができません。

2chの 632の件において文字間隔が 1 のときに改行を CRLFにすると問題が隠れるのは、634で引用されている「//※改行コードの文字数を文字幅と見なす」というコメントが示唆するとおり、最後の非改行文字+2 のX座標を LayoutToLogicExに与えているがためにループの終了条件をうまくくぐり抜けているのです。+2 が +100 でも問題が起こらないので見逃されていたミスだと思います。

パッチでは、LogicToLayoutにおいては改行文字の幅に文字間隔をプラスした座標を返します。LayoutToLogicExにおいては、ある文字と文字間隔から 1レイアウト単位でも右にはみ出していれば次の文字をポイントするように条件判断の位置と境界条件(=の有無)を変えました。改行付近にしか影響はないはずです。

どちらか片方の修正だけでも一般掲示板[8087]は解決します。2chの 632の件には LayoutToLogicEx の修正が必要です。

あまりに基礎的な関数のため影響が読み切れません。不安です。上記の文章にもしれっと嘘が混じっている可能性が払拭しきれません。

[Moca]

あんまりいい修正方法ではないかもしれませんが、「レイアウトの改行は必ず1になる」ように
修正する案を置いておきます。
利点は、元からあるあらゆる改行位置をはさんだ比較文のところを再チェック・修正せずに
つかえそうな気がするところです。

http://mocaskr.web.fc2.com/weekly/sakura_weekly.html
sakura2-3-0-0r4055_update2.zip に適用してあります。
あと直接関係ありませんが、なぜかうちのgithub版では$のほうの問題は起きないようです。謎です。

[ds14050]

そうですよね、文字間隔の部分で対応する方が影響は少ないですよね。

サクラエディタふぁんくらぶ part17の問題では、どの部分の変更で対応するか迷ってしまい、決定版の対応策を出せなかったのですが、そのときも GetLayoutXOfChar をいじる案は思いつかなかったですねえ。

理由は自分の傾向にあるとわかっています。ほとんど成立しないレアケースを見張るために if 文を埋め込むというのが感覚的に嫌なんですよね。

影響が少ない方がいいと思います。

ちなみに、GitHub版では Command_REPLACE_coreが Command_INSTEXTに処理を受け渡すまでに、改行の文字幅(1)に文字間隔をプラスしたレイアウト範囲が選択されています。だからその後の置換処理において改行文字に上書きするように改行が挿入されます(これが正常な動作)。最初の投稿に書いたように LayoutToLogicExでループの終了条件を早期に満たしてしまうかどうかの境目が文字間隔のあるなしです。

Command_REPLACE_core → LogicToLayout → CalcLayoutWidth → GetLayoutXOfChar と処理を深掘りしていくと CLayoutMgr.h の中の GetLayoutXOfChar が本家と GitHub版で違っていました。違いはそこなのですが、その一つ前、CLayout::CalcLayoutWidth が「レイアウト幅を計算。改行は含まない。」という関数なのですが、これがちょっとおかしいんですよね。冒頭で

nTextLen = m_pCDocLine->GetLengthWithoutEOL();

と改行なしの長さを取得しておきながら、メインのループでは

CLogicInt nLen = GetLogicPos().x + m_nLength; //EOL=0,1
for(CLogicInt i=m_ptLogicPos.GetX2();i<nLen;){

と、EOLの長さが 0 か 1 として含まれる nLen を使っています。

改行文字を対象とした GetLayoutXOfChar の値の違いが GitHub版の CalcLayoutWidth の嘘(改行文字の幅とその後ろの文字間隔を含めている)の理由ですが、改行文字の幅を足そうとしている CalcLayoutWidth がそもそも間違っていると言えます。

[Moca]

github版のほうの解説ありがとうございます。
GetLayoutXOfChar で nLineLen < i のときの動作が違うんですね。
github版はとりあえず動いてますが、コードが正しいとは言えないですね。

rev1.patch のLayoutToLogicExで中途半端なレイアウト座標の変換を「切り上げ」に仕様変更するのは、
矩形選択の文字を半分だけ選択した場合の座標変換などで問題があると思います。
そこはLineColumnToIndexと動作を合わせる必要もあります。

やるなら「改行文字をCLayoutXInt(1)とする」と直接コードで書かれている側を修正して
改行==「1 + cLayoutMgr.GetCharSpacing()」になるように変更して、
必要なら追加で各所の比較などもそれに合わせて変更する必要が出てきます。
２ｃｈで示された最初の修正案に近いものです。
添付パッチ(type2)みたいな感じです。

あと細かい所ですがstd::maxはサクラではt_maxに揃えることになってるのでよろしくです。

[ds14050]

揚げ足取りだと嫌わないでくださいね。これが習い性ですし CPUを相手にするのに好都合だとも思っています。nLineLenではなく nDataLen ですし、nDataLen <= i の場合に、文字幅(0)+文字間隔を返すか(GitHub版)、文字幅(0)+(半角単位の文字幅(0)*文字間隔)を返すか(本家)という違いです。小なり(大なり)イコールです。

車の運転が典型なのですが、自分は脱輪したりこすりそうなとき、目をつぶってえいや、で行ってしまう傾向があるようです。教習中に気がつきました。なので、矩形選択に影響がでるだろうかとかそういうことを洗い出したり影響を検討したりせずに、とりあえずやってみるわけですね。それが rev1.patch です。

実際のところ矩形選択に影響はあるでしょうか？ パッチが関係するのはループを回りきったとき、改行がからむ場面だけで、普通に文字がある場所に影響があるようではパッチのバグです。

いつのまにか増えていた上書きモードのオプション「☑矩形入力で選択範囲を削除する」と「□改行コードは上書きしない」を組み合わせたときに、改行コードが選択される・されないが影響する気がしたのですが、確かめようにもオプションが設定できませんでした>>一般掲示板[8092]。

sakura_core/view/CEditView_Command_New.cpp がコンパイルできません。自分はよくわからないので、2chでは else節からコピペしてきました。あと、ここで EOLの長さ(TCHAR 1個か2個)に応じた文字間隔を加算するのはおそらく間違いです。「//※改行コードの文字数を文字幅と見なす」という古いコメントが見えますが、レイアウト座標系において改行はいつでも幅1ではありませんか？ そこに足す文字間隔は半角１個に対する量で十分だと思います。

コメントによると t_maxを最初に書いたのは kobakeさんでしたか。テンプレート版を明示する目的で。そこは、あえて std::maxを使う理由はありませんが、あえて t_maxを使う理由もありませんので std::maxと書きました。然るべくお読み変えください。

テストがないサクラエディタで変更に対して保守的になってなりすぎることはないと思っています。そういう意味で、rev1.patchに固執しているように見えたらそれは誤解です。

[ds14050]

type2パッチベースで [r4196] としてコミットしました。

改変点はコンパイルエラーを取り除いたことと、CRLFに
対して２文字分の文字間隔を与えないようにしたことだけです。