#1113 矩形選択置換が範囲を無視して置換してしまう

[ds14050]

掲示板に報告があった件です。
http://sakura-editor.sourceforge.net/cgi-bin/cyclamen/cyclamen.cgi?log=data&v=8292#8292

「単位混在」の注意喚起があった部分が原因で、起こるべくして起こったなと、これまでよく動いていたなという感じでしょうか。これは取りあえずの穴を塞ぐ、文字通りのパッチです。

---

補足条件は正規表現検索を行うことです。発生はバージョンは 2.2.0.1と 2.3.0.0の間からそれ以降。「「すべて置換」は置換の繰返し」のチェック状態は問いませんが、コードパス(直接の原因)は分かれています。

---

パッチの注釈です(上から順番に)。

1. colDiffは colDifと名前が紛らわしいため、変更の上スコープを狭めました。
2. 一括置換なので ptOldの x座標を更新する必要はないと考えました(次のループは次の行へ行くだろう)。これがチェックなしの場合の直接の原因です。
3. exTailが colDiffの新しい名前です。
4. スペースインデントをタブに変更したために変更範囲が大きな塊になっています。
5. matchLenは冗長に見えたので省きました。
6. 「無限置換にならないように１文字進める」部分の if (nIdxTo < nLen) は一見不要に思えますが残しました。assertでどうかとも思いましたが if のままです。
7. キャレット位置の調整に伴う選択範囲の変更は、nIdxToのみに依存するようにして２か所を１つにまとめました。
8. ptOldを更新する際に bBeginBoxSelect を考慮するようにしました。フラグにより ptOldを含む複数の変数の意味が変わります(単位混在の原因)。これでチェックありの場合が直ります。
9. ptOldにセットする値は２行上で更新されていたものが手近だったために利用しました。「行位置も取得する」というコメントがあったため y 座標とまとめて代入しています。bFastModeは、範囲置換の場合(bBeginBoxSelect==trueを含む)にはオフのようなので、必要な値が(２行上で)得られないことはなさそうです。

[ds14050]

掲示板で ark さんが挙げたケースを試すマクロです。なるべくまっさらな ini で、正規表現が利用可能な状態で、無題テキストを対象に実行するのが吉。

文字数や選択位置により必ず起こるわけではないために、できるだけ多くの例を挙げられたのだと思います。

しかしこれはバグ報告の役には立ってもデグレを検知する目的には使えないんですね。テストの、蓄積が、ない！

---

2017-11-28 20:41:06

BugReport/70 と BugReport/192 のケースを追加しました。

[#1028]矩形選択置換で置換範囲がおかしいバグの修正の _v3パッチ版はすべて通ります。反対に、比較的古いバージョン(2.2.0.1)で失敗が見つかるようになりました。

[ds14050]

正直なところより広範囲をターゲットにした [#1028] が早くから存在していたことでこのチケットの意義はどこかへ行ってしまいましたが、パッチのアップデートです。2017-11-28 20:41:06 にアップデートされたテストマクロがすべて通るようになります。

注釈

2.一括置換なので ptOldの x座標を更新する必要はないと考えました(次のループは次の行へ行くだろう)。これがチェックなしの場合の直接の原因です。

というのが嘘だったので

8.ptOldを更新する際に bBeginBoxSelect を考慮するようにしました。フラグにより ptOldを含む複数の変数の意味が変わります(単位混在の原因)。これでチェックありの場合が直ります。

部分のコードブロックを移動して、両方の場合をカバーするようにしました。

収支は +33行/-39行です。

[ds14050]

欲張って[#1028]の領域までカバーしようとしたパッチ(第３版)です。

新規部分は前半に集中しています。予想されたことですが(※「とりあえずの穴を塞ぐ」ことに徹した理由)、付け焼き刃でどうにかなるとは思われず、特定の１コードブロックに限ったことですがごそっと書き換わっています。

現在までに自分がここと[#1028]に投稿した３種のテストマクロがすべて通るようになります。パッチ内のコメントでインデントに言及していますが、折り返し行インデントがあっても動いたらいいなあという期待があるだけです。２番のケースのテストがまだありませんので(※実例が見つからない)、これに絞った対応は何もしていません。

今回で収支は -63行/+79行になりました。

[ds14050]

矩形選択範囲置換という共通点があるのでここに投稿します。テストファイル名の通りですが、1.6.6.0, 2.2.0.1, 2.3.2.0は全滅です。[#1028]の _v3パッチ版はすべて通ります。どちらを期待すべきかわかりません。

[ds14050]

パッチの第４版です。直前のコメントの「行頭から検索を繰り返すと矩形選択範囲内でマッチが見つけられないケース」に対応しました。具体的には _v3パッチから「2016.01.13 範囲選択をクリアしないと位置移動できていなかった」とコメントされていた１行をパクってきました。(こういう効果があったのですね)

選択した範囲を対象にして置換するというとき、自分は今まで選択範囲だけを抜き出したような状態で検索・置換を行っているのだと想像していました。ですので選択範囲の左端から検索が始まることは期待に沿っています。

---

今後

「2. 置換文字の右側にタブがあった場合の置換範囲がおかしかったのを修正」のテストケースが見つかるか、見つからなくても今の勢いのまま(フェードアウトしてしまう前に)コミットしてしてしまおうと思っています。_v3パッチも完全ではありませんし、他人のパッチを本人になりかわって育てることはできませんので。パッチの影響範囲は矩形選択範囲置換のみに限られています(そのはず)。文句をつけたい方は機会を逃さないようご注意ください。

[ds14050]

パッチの第５版です。[#1028]の方へ投稿した「矩形選択範囲置換で半分はみ出している文字を置換するケース.js」に対応しました。

注釈

GetSelect().GetTo() と GetSelect().GetFrom() を ptNew と ptNewFrom に置き換えたために変更が広範囲に渡っているように見えますが、実質的な変更点は次の２点です。

• raggedLeftDiff 変数の導入 (firstLeftの補正に使われています)
• 「次の検索開始位置へシフト」する座標をマッチ終端基準からマッチ先頭基準へ (_v3パッチのパクりです。実際にこれが問題になるテストケースは作れませんでしたが、正規表現検索はマッチ長さが可変なので不可能ではない気がします)

収支は -63行/+93行です。(9行のまとまったコメントを新しく書きました)

[ds14050]

パッチのバグ修正(3点)と小改良(1点)です。すべて r3 から存在するものです。

• firstLeft を計算する際に colDif を算入していました。colDif は矩形範囲左端には影響しません。
• 「折り返し直前まで選択した場合は選択範囲が次行行頭を含む２レイアウト行にまたがることに注意が必要」とした部分で、括弧が抜けていたために条件が必要なところまで分配されていませんでした。実質的な影響(意味の違い)はありません(怪我の功名)。
• 「選択範囲置換の場合は行内の選択範囲末尾まで置換範囲を縮める」部分で不要な書き換えをして意味を変えてしまっていました。これにより改行が置換できなくなっていました。テストケースを添付します。
• (小改良) colDif の更新を早めて colDif を利用する際の条件判断を省きました。

[ds14050]

パッチの第６版です。最後の懸案の、タブの幅がその出現位置により変化することへの対応です。_v3パッチのアイディアと実装をパクりました。これまでに自分がこちらと[#1028]へ投稿したテストマクロはすべてとおります。

収支は -70行/+107行です。

いろいろなケースに対処して一貫性のある結果を引き出すために四苦八苦した結果とはいえ、ひどいもんですね。

備考

正規表現一括置換の場合を除いてこの版と _v3パッチ版に共通する挙動で、既存のバイナリとは違う挙動があります。

矩形範囲が改行よりも右側へ広がっているときに、改行がなくなり次行が連結された場合、既存バイナリはレイアウト座標を基準にしているため連結された次行(※矩形範囲に入っていなかった部分と入っていた部分が含まれます)が検索置換の対象になります。パッチ版は最初に計算した(その変換により切り詰められた)ロジックX座標に基づくためにそれらは対象になりません(※確実に、最初は範囲に入っていた部分が漏れます)。

「結果は未定義」でも問題がないケースだと思っています。マクロを添付します。

余談

ptOld からはじまる「単位混在」とマークされた変数群は Layout と Logic にきれいに分けられます。つまり本質的に混じり合わないコードが折り重ねられ、たまたま似たような目的の変数をいくつか共通に使用していたために、変数の記憶領域のみが共用されています。これを２つのクラス(ファンクタ)に分離して正常化しつつ、記憶領域が重複したり、記憶領域がスタックではなくヒープに置かれるといったデメリットを避ける、普通の(凡人が扱える)方法がないというのは C++ の欠陥ではないかと思います。C++11 で一部の制限が取り払われたらしい共用体が答えになりそうな気がしていますが、2010より新しい Visual Studio がインストールできる Windows がないのでした。

[ds14050]

[r4201] でコミットしましたが文字化けしています。アップデートした Subversionクライアントのインストールが不完全だったせいだと思います。コミットメッセージには

Fix: 矩形選択置換が範囲を守っていなかった。
[patchunicode:#1028] by Moca
[patchunicode:#1113] by ds14050

と書きました。この文字列を表す UTF-8 バイト列をそのまま Shift_JISと解釈したような化けかたです。リポジトリには UTF-8 文字列を強制的に Shift_JIS->UTF-8 変換したようなバイト列が保存されているのではないかと思います。ともあれ申し訳ありません。

[novice123]

svn logは修正しましたが、web[r4201]のほうは更新されないです。