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.

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

正規表現コンパイルエラー時の日本語エラーメッセージ追加 #1935

Merged
merged 2 commits into from
Sep 16, 2023

Conversation

tats-u
Copy link
Contributor

@tats-u tats-u commented Sep 13, 2023

PR対象

  • アプリ(サクラエディタ本体)

カテゴリ

  • 改善

PR の背景

#1780 の緩和

仕様・動作説明

現在は鬼雲から返ってきた英語のエラーメッセージをそのまま表示しているが、直前に「正規表現に誤りがある」という旨の日本語共通メッセージを追加するように変更。

文字列の結合には、std::wstring + +=を使用。

PR の影響範囲

正規表現を利用した検索

テスト内容

文書内検索時に以下のことをして検索

  • 正規表現をオン
  • 検索文字列を誤りのある正規表現にする

関連 issue, PR

#1780

参考資料

image

@tats-u tats-u changed the title 正規表現コンパイルエラー時のエラーメッセージ追加 正規表現コンパイルエラー時の日本語エラーメッセージ追加 Sep 13, 2023
@AppVeyorBot
Copy link

Build sakura 1.0.4311 completed (commit b6da765155 by @tats-u)

@beru
Copy link
Contributor

beru commented Sep 13, 2023

動作未確認ですが、英語用のリソースファイル (sakura_lang_en_US/sakura_lang_rc.rc) についても STR_BREGONIG_PREAMBLE の文字列リソースを追加すると良いのではないかと思いました。

@AppVeyorBot
Copy link

Build sakura 1.0.4312 failed (commit f1d97ef0cd by @tats-u)

@AppVeyorBot
Copy link

Build sakura 1.0.4313 completed (commit bea83e0568 by @tats-u)

@AppVeyorBot
Copy link

Build sakura 1.0.4314 completed (commit b1a6458ae0 by @tats-u)

sakura_core/sakura_rc.rc Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

Build sakura 1.0.4315 completed (commit cdeffc9f92 by @tats-u)

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.

PRありがとうございます。提案内容はいいと思います。

書きっぷり関連でいくつか指摘あげたので、修正or申し開きの対応をお願いします。

@@ -1326,5 +1326,6 @@
#define STR_FILEDIALOG_MRU 35040
#define STR_FILEDIALOG_OPENFOLDER 35041
#define STR_GSTR_APPNAME 35047
#define STR_BREGONIG_PREAMBLE 35048
Copy link
Contributor

Choose a reason for hiding this comment

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

シンボル名が微妙な気がします。

bregonig = 正規表現DLLの固有名
preamble = 「前文」という意味の英単語

「これなんですか?」の解答にならない名前は微妙な気がします。

既存でとんでもない名前付いてるやつ(ファイル名の連番とか)もあるのでスルーしてもよいのですが一応。

Copy link
Contributor Author

@tats-u tats-u Sep 16, 2023

Choose a reason for hiding this comment

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

多少長くなっていいのならSTR_REGEX_COMPILE_ERR_PREAMBLEでどうですか?


// Now using max number 35047 by STR_GSTR_APPNAME
// Now using max number 35048 by STR_GSTR_APPNAME
Copy link
Contributor

Choose a reason for hiding this comment

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

連番振ってくならこの行ムダですよね。
(指摘じゃないので修正の必要はありません。)

Copy link
Contributor

Choose a reason for hiding this comment

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

と思ったけど、こうじゃないですかね(利用シンボル名も書き替える。)

Suggested change
// Now using max number 35048 by STR_GSTR_APPNAME
// Now using max number 35048 by STR_BREGONIG_PREAMBLE

sakura_core/extmodule/CBregexp.cpp Show resolved Hide resolved
@@ -30,12 +30,12 @@ LANGUAGE LANG_ENGLISH, SUBLANG_ENGLISH_US
// TEXTINCLUDE
//

1 TEXTINCLUDE
1 TEXTINCLUDE
Copy link
Contributor

Choose a reason for hiding this comment

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

変更理由があれば教えてほしいです。
変更理由がなければ関係ない変更をrevertするコミットを追加してほしいです。

Copy link
Contributor Author

@tats-u tats-u Sep 16, 2023

Choose a reason for hiding this comment

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

意図しない変更です(NeoVimの?末尾空白の自動削除機能の模様)
Markdown以外で末尾空白に意味を持たせてるのは初めて知りました。
何か意図はありますか?
意図しないなら(今回は消しませんが)Markdown以外タイミングを見て一括削除すべきかと思います。
#786 が解決したら気にする必要がなくなりますが)

@tats-u tats-u force-pushed the regex-error branch 2 times, most recently from 41bcb8d to 56b8a61 Compare September 16, 2023 02:44
@AppVeyorBot
Copy link

Build sakura 1.0.4316 completed (commit 7caec8641e by @tats-u)

@AppVeyorBot
Copy link

Build sakura 1.0.4317 completed (commit 3b55ff3389 by @tats-u)

@tats-u tats-u force-pushed the regex-error branch 2 times, most recently from a387da7 to 60cf673 Compare September 16, 2023 03:47
@AppVeyorBot
Copy link

Build sakura 1.0.4318 failed (commit 909d182cf7 by @tats-u)

@AppVeyorBot
Copy link

Build sakura 1.0.4319 completed (commit 81b2cca56f by @tats-u)

@sonarcloud
Copy link

sonarcloud bot commented Sep 16, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@tats-u
Copy link
Contributor Author

tats-u commented Sep 16, 2023

翻訳対応の文字列を追加すると#defineするなとSonaCloudがうるさいですね。
今後どうにかすべきかと思います。

@AppVeyorBot
Copy link

Build sakura 1.0.4320 completed (commit a6bb60106c by @tats-u)

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.

問題なさそうに思います。

(変更後のシンボル名が長いのが若干気になりましたが問題ないと思います。)

@berryzplus
Copy link
Contributor

翻訳対応の文字列を追加すると#defineするなとSonaCloudがうるさいですね。 今後どうにかすべきかと思います。

SonarQube側の誤検知(false positive)と考えられるのでそのようにマークしときました。

@beru beru merged commit d083650 into sakura-editor:master Sep 16, 2023
17 of 20 checks passed
@tats-u tats-u deleted the regex-error branch September 16, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ■機能追加
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants