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

CSearchAgent::ReplaceData のテストを追加する #1661

Merged

Conversation

kengoide
Copy link
Member

@kengoide kengoide commented May 8, 2021

PR の目的

CSearchAgent::ReplaceData に対する自動テストを追加します。

カテゴリ

  • その他の問題

PR のメリット

  • コードカバレッジが向上します。

仕様・動作説明

  • 例によって bEnableExtEol フラグへの参照を関数外に追い出す変更を行います。
  • 細かな最適化コードに対するカバレッジが完全ではありません。
  • 実行条件が判然としなかった一部のコードパスをカバーしていません。

カバレッジの不足については追々対処していきます。

PR の影響範囲

CSearchAgent::ReplaceData と呼び出し元のコードに影響します。

@AppVeyorBot
Copy link

Build sakura 1.0.3743 completed (commit ccfbda1faa by @k-kagari)

@berryzplus
Copy link
Contributor

CodeSmells

Refactor this function to reduce its Cognitive Complexity from 212 to the 25 allowed.
https://sonarcloud.io/project/issues?id=sakura-editor_sakura&issues=AXlMZCJDHdxPSVS0PlFK&open=AXlMZCJDHdxPSVS0PlFK&pullRequest=1661

対応不能で異論ないです。

Make the type of this variable a pointer-to-const. The current type of "pcGrepAgent" is "class CGrepAgent *".
https://sonarcloud.io/project/issues?id=sakura-editor_sakura&issues=AXlMZCJDHdxPSVS0PlFL&open=AXlMZCJDHdxPSVS0PlFL&pullRequest=1661

const付けてくださいって言ってますね。
対応するのがベターだと思います。

Use the init-statement to declare "pcGrepAgent" inside the if statement.
https://sonarcloud.io/project/issues?id=sakura-editor_sakura&issues=AXlMZCJDHdxPSVS0PlFJ&open=AXlMZCJDHdxPSVS0PlFJ&pullRequest=1661

if (const auto* pcGrepAgent = CEditApp::getInstance()->m_pcGrepAgent; pcGrepAgent &&にしてくださいって言ってますね。
対応するのがbetterだと思います。

@kengoide kengoide force-pushed the feature/tests-for-replacedata branch from 6e2c67d to c61a2f1 Compare May 8, 2021 14:47
@sonarcloud
Copy link

sonarcloud bot commented May 8, 2021

@kengoide
Copy link
Member Author

kengoide commented May 8, 2021

Code Smells に対応しました。

@AppVeyorBot
Copy link

Build sakura 1.0.3744 completed (commit 381f3bd5c5 by @k-kagari)

@berryzplus
Copy link
Contributor

  • 例によって bEnableExtEol フラグへの参照を関数外に追い出す変更を行います。

かなり今更感あるんですけど、bEnableExtEol フラグを取得するためのシングルトン(ないし、シングルインスタンス)を用意したらパラメータ変更してまわらなくても良かったですよね・・・。

たぶんbEnableExtEolはエディタのプロパティで、
サクラエディタのプロセス内には必ずエディタが存在するので、
この値を取得するためにエディタのインスタンスにアクセスする仕様にしたらよい気がします。

今回突然やりましょう!ってのはおかしいので、今後どこかでやれたらいいなと思っています。

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.

良さそうに思います。

@kengoide
Copy link
Member Author

kengoide commented May 9, 2021

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

bEnableExtEol については考えられる進め方が複数あって迷いますね…。当面は別件としておきます。

@kengoide kengoide merged commit b670708 into sakura-editor:master May 9, 2021
@kengoide kengoide deleted the feature/tests-for-replacedata branch May 9, 2021 05:02
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.

None yet

3 participants