Skip to content

GitHub #2476 APIが正常値の戻り値0を返すとき最新のエラー情報をクリアしない#2490

Closed
uzanka wants to merge 1 commit into
sakura-editor:masterfrom
uzanka:feature/check-by-copilot
Closed

GitHub #2476 APIが正常値の戻り値0を返すとき最新のエラー情報をクリアしない#2490
uzanka wants to merge 1 commit into
sakura-editor:masterfrom
uzanka:feature/check-by-copilot

Conversation

@uzanka

@uzanka uzanka commented Jun 16, 2026

Copy link
Copy Markdown

PR対象

  • アプリ(サクラエディタ本体)
  • テストコード

カテゴリ

  • 改善
  • 不具合修正

PR の背景

#2476

仕様・動作説明

空文字列が設定されているテキストボックスの文字列をApiWrap::Wnd_GetText()で取得したとき、
正常値としての戻り値0とエラーとしての戻り値0の区別をするためにGetLastError()でエラー判定しているが、
GetWindowTextLength()関数はエラーが発生しない場合に最新のエラー情報をクリアしない。
本関数を呼び出す前にSetLastErrorでクリアすることで、エラー判定を正常に動作するようにする。

PR の影響範囲

GetWindowTextLength()関数がエラーとなるのは主にウインドウハンドルが不正(NULL)な場合であるため、
サクラエディタのソースでは問題となるルートには入らないと予想される。

テスト内容

  • SetLastError(1)でエラー情報をセットした状態で、正常値0が返る空文字列を取得する。
  • ウインドウハンドルにNULLを指定して、異常値0が返るエラールートを通す。

関連 issue, PR

参考資料

GetWindowTextLength
https://learn.microsoft.com/ja-jp/windows/win32/api/winuser/nf-winuser-getwindowtextlengthw

@github-actions

Copy link
Copy Markdown

Test Results

1 098 tests  ±0   1 098 ✅ ±0   4m 53s ⏱️ +10s
  103 suites ±0       0 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 4c52055. ± Comparison against base commit 71514c3.

@sonarqubecloud

Copy link
Copy Markdown

@berryzplus berryzplus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetLastErrorする必要性に疑問があるので、修正に消極的です。

GetWindowTextを呼ぶ目的:
「ウィンドウに設定されているテキスト」を取得したい。

期待する結果:
「ウィンドウに設定されているテキスト」が取得される。

取得した値をどう使うか(≒取得した値がおかしかった場合にどう報告するか)を考えると「値を取得できませんでした。」でなく「入力値が空です。」になる気がします。

文字列長がWindowsのエラーで取れなかった場合とテキスト未入力で取れなかった場合を区別する必要があるかどうか。自分は「その必要はない」ように思ってます。

あと、SetLastErrorはグローバルなAPIエラー状態を更新してしまうので、直し方としてあまり良くない気がしています。ウィンドウのテキスト取得が「グローバルなAPIエラー状態に依存する処理である」とは思えないので。

@uzanka uzanka closed this by deleting the head repository Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants