Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CDlgInput1で文字列長はNUL終端を含まない長さを指定するはずが1文字多いところがあるのを修正 #1589

Merged
merged 1 commit into from
Mar 15, 2021
Merged

CDlgInput1で文字列長はNUL終端を含まない長さを指定するはずが1文字多いところがあるのを修正 #1589

merged 1 commit into from
Mar 15, 2021

Conversation

usagisita
Copy link
Contributor

PR の目的

タイトルの通り、1行入力ダイアログのいくつかの画面に、1文字分長い文字列制限値を指定しているため、入力画面で限界まで文字を入れると、落ちたり落ちなかったり、動作不良だったり、ちょっと不健康です。
このPRでは、不具合として修正します。

カテゴリ

  • 不具合修正

PR の背景

異常終了、不安定な動作はなるべく直したいです。

PR のメリット

バグが減ります。

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

特にありません。

仕様・動作説明

履歴ダイアログの「検索」などの文字列はダブルクリックか右クリックで「編集」できます。
同、左下の「追加」や右メニューの「新規追加」でも入力画面が出ます。
同じような感じで、プロファイルマネージャの新規作成、名前変更があります。
ファイルツリー設定画面の「パスの一括置換」も同上です。
印刷プレビューのページ番号の「...」の直接指定のダイアログもです。
以上が、1文字多くなっていたところです。

なおCDlgPrintSettingのCDlgInput1の変数は未使用だったので削除しました。

PR の影響範囲

上記、ダイアログの入力可能文字数が1文字減ります。

テスト内容

手順
元のコードベースで実行し、履歴ダイアログで新規追加はMAX文字入力すると、履歴に追加されません。
PRのコード修正を適用すると、MAX文字(前より1文字少ない)でも追加されます。

テスト1

結合テスト的なものなので、単体テストは書くのが難しいため、ありません。
サクラエディタを実行し、各ダイアログ上を操作して、正常動作するか確認します。

関連 issue, PR

参考資料

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

✅ Build sakura 1.0.3554 completed (commit c4a83b1ee7 by @usagisita)

berryzplus
berryzplus approved these changes Mar 14, 2021
View reviewed changes
Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

発生している事象への対処として適切だと思います。

本質的には CDlgInput1 が提供するインターフェースが分かりづらいことが元凶だと思ったので、別件としていつか対策したいです。

@sanomari
Copy link
Contributor

PR内容は問題ないと思います。

本質的には CDlgInput1 が提供するインターフェースが分かりづらいことが元凶だと思ったので、別件としていつか対策したいです。

前に作成して放置中のコードなんですが、こんなの作りました。
https://github.com/sanomari/sakura-editor/blob/0b3583fbc8cd245584bf287c72e5f07995bf729b/sakura_core/dlg/CDlgProfileMgr.cpp#L55-L66

@usagisita
Copy link
Contributor Author

リファクタリングをともわない、どんなバグかわかる最低限度の修正ということで、ここはひとつ。

sanomari さんのコードも見ました。ダイアログの処理と、プロファイルマネージャの機能の分離、いいと思います。
コンフリクトしてしまうと思いますが、甘えさせてもらって、先にこれは入れさせてもらおうと思います。

確認ありがとうございました。

@usagisita
Copy link
Contributor Author

マージします。

@usagisita usagisita merged commit 95d9b03 into sakura-editor:master Mar 15, 2021
@usagisita usagisita deleted the feature/fix_dlginput1_text_length branch March 15, 2021 15:25
@berryzplus
Copy link
Contributor

前に作成して放置中のコードなんですが、こんなの作りました。
https://github.com/sanomari/sakura-editor/blob/0b3583fbc8cd245584bf287c72e5f07995bf729b/sakura_core/dlg/CDlgProfileMgr.cpp#L55-L66

むしろコミットメッセージが気になる感じの修正でした。
　👉 プロファイルマネージャの機能をダイアログから分離する

そろそろ gmock をオンラインにするPRを作成すべきなのかも知れない、と思いました。

@beru beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@berryzplus berryzplus berryzplus approved these changes

Assignees
No one assigned
Labels
🐛bug🦋 ■バグ修正(Something isn't working)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@usagisita @AppVeyorBot @sanomari @berryzplus @beru