タイプ別設定の「カーソル位置縦線」を有効時に、カーソルが先頭行にある状態で PageUp キーを押した後にカーソル左右移動するとカーソル位置縦線が残る問題を修正

[beru]

カーソルが最終行時の PageDown キー後のカーソル左右移動についても同様です。

PR の目的

件名の通りです。

カテゴリ

• 不具合修正

PR の背景

#1617 で不具合報告のIssueが作成されました。
どの時点で不具合が入ったのかを調べたところ #1320 の変更が原因という事が判明しました。

PR のメリット

件名に記載した不具合が修正されます。

PR のデメリット (トレードオフとかあれば)

判定条件が追加されてコードが読み取りづらくなります。

仕様・動作説明

前のカーソル位置縦線の消去が行われない問題が起きるメカニズムを調べました。

先頭行でPageUpキーを押した際に下記のコールスタックで呼び出しが行われます。

>	sakura.exe!CEditView::CaretUnderLineOFF(bool bDraw, bool bDrawPaint, bool bResetFlag, bool DisalbeUnderLine) Line 2534	C++
 	sakura.exe!CCaretUnderLine::CaretUnderLineOFF(bool bDraw, bool bDrawPaint, bool bResetFlag) Line 97	C++
 	sakura.exe!CCaret::MoveCursor(CStrictPoint<SLayoutPoint,CStrictInteger<1,1,1,0,1>,CStrictInteger<1,1,1,0,1>> ptWk_CaretPos, bool bScroll, int nCaretMarginRate, bool bUnderLineDoNotOFF, bool bVertLineDoNotOFF) Line 200	C++
 	sakura.exe!CCaret::Cursor_UPDOWN(CStrictInteger<1,1,1,0,1> nMoveLines, bool bSelect) Line 975	C++
 	sakura.exe!CViewCommander::Command_1PageUp(bool bSelect, CStrictInteger<1,1,1,0,1> nScrollNum) Line 750	C++

CEditView::CaretUnderLineOFF メソッドで引数の bDraw の値が false な為、カーソル位置縦線の消去は行われず CEditView::m_nOldCursorLineX の値が -1 に設定されます。

その後のカーソル左右移動時の CEditView::CaretUnderLineOFF メソッドでは、 CEditView::m_nOldCursorLineX の値が -1 に設定されている為、前のカーソル位置縦線の消去が行われません。

#1320 の変更前は CViewCommander::Command_1PageUp から呼び出される CEditView::RedrawAll の呼び出し以降の処理で画面全体の再描画を行う際に CEditView::CaretUnderLineON が呼び出されてそこで CEditView::m_nOldCursorLineX の値が -1 ではない値に設定される為、その後のカーソル左右移動時に問題が発生しませんでした。

このPRでは、カーソル位置縦線を描画する設定の場合には、PageUp, PageDown 時の CEditView::RedrawAll 呼び出しを省かないように判定を追加する事で問題を回避しました。

PR の影響範囲

PageUp, PageDown キー押し時の描画内容

テスト内容

テスト1

手順
タイプ別設定の「カーソル位置縦線」有効時に、カーソルが先頭行にある状態でPageUpキー、もしくは最終行にいる状態でPageDownキーを押した後にカーソル左右移動をしても前の「カーソル位置縦線」の表示が残らない事を確認

関連 issue, PR

#1320, #1617

[beru]

ちょっと対処が場当たり的というか、色々なケースに応じて条件判定を追加する形で対応しているので下記のような懸念があります。

• 色々な条件判定の記述が追加される事で本来の目的の為の処理が読みづらくなる
• 条件追加をして対処したが、まだ気付いていないケースで正しく描画が行われない可能性があるかもしれない
• 将来的に何か別の描画を追加したい場合に、ここらへんの条件判定を行って描画を省く記述が仇となって追加が困難になる

まぁ記述がごちゃごちゃしていて可読性が低いのは元からですが、そのコードをリファクタせずにやっつけの変更のような形で処理を追加しているので、可読性が低い問題を悪化させている感じがあります。

[AppVeyorBot]

✅ Build sakura 1.0.3655 completed (commit cdf148b1cb by @beru)

[berryzplus]

指摘するかどうか迷いましたが、書いておきます。

条件式はtrueを判定するほうが読みやすいらしいです。
bNoCursorVLine（＝タイプ別設定のカーソル位置縦線が無効）を定義して使うほうが若干読みやすくなるはずっす。

[beru]

確かにそうかもしれないので af457d3 で修正しました。

このPRの変更ではなく自分が以前行った #1320 の変更によるものですが、CViewCommander::Command_1PageUp と CViewCommander::Command_1PageDown で同じような記述を書いているのが良くないかなと思います。違いといえば nScrollNum の符号ぐらいだと思うので private method を作って記述の共通化を図ったような良い気がしてきました…。

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[AppVeyorBot]

✅ Build sakura 1.0.3659 completed (commit 4b66511051 by @beru)

[beru]

レビューありがとうございました。Mergeします。