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

strprintfを改良してナロー文字も扱えるようにする #1810

Merged
merged 5 commits into from
Feb 27, 2022

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Feb 24, 2022

PR の目的

タイトル通りです。
独自関数 strprintf を std::string でも使えるようにします。
これは #1722 の対策に必要と考えています。

カテゴリ

  • その他の問題
    独自関数 strprintf の機能を拡充します。
    サクラエディタの機能は増えないので、機能追加ではありません。

PR の背景

#1722CPPA の不具合が報告されています。

修正したいのですが、CPPA は narrow 文字列を扱う機能なので、現状の strprintf だと対応に困ってしまいます。

対策として、std::wstring のみに対応していた strprintf を、
std::stringでも扱えるように改良したいと思います。

PR のメリット

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

仕様・動作説明

アプリの仕様・機能に影響を与える変更ではありません。

WCHAR版を以下の4オーバーロードに組みなおし、同機能のCHAR版を追加します。

  • int vstrprintf(std::wstring& strOut, const WCHAR* pszFormat, va_list& argList) 確保済み文字列を受け取る高速版(va_list)
  • int strprintf(std::wstring& strOut, const WCHAR* pszFormat, ...) 確保済み文字列を受け取る高速版(可変長引数)
  • std::wstring vstrprintf(const WCHAR* pszFormat, va_list& argList) 出力先指定不要の簡易版(va_list)
  • std::wstring strprintf(const WCHAR* pszFormat, ...) 出力先指定不要の簡易版(可変長引数)

PR の影響範囲

ユーティリティ系関数の変更なので、あちこちに影響します。

テスト内容

基本的な機能確認を行えるようにテストケースを 2件 3件 追加しています。
CIビルドが通れば基本的な機能確認は問題ないと考えられます。

テスト1

手順

関連 issue, PR

#1722

参考資料

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@berryzplus
Copy link
Contributor Author

CIの結果とカバレッジを見て追加コミットを積みました。

SonarCloud が検出した以下の Code Smells については対応しません。

@AppVeyorBot
Copy link

@berryzplus berryzplus marked this pull request as ready for review February 25, 2022 00:07
Copy link
Contributor Author

@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.

難しい修正のつもりはありません。
なんでレビュー付かないんですかね?

セルフレビューで改善点に気付いたので対応しようと思います。。。

sakura_core/util/string_ex.cpp Outdated Show resolved Hide resolved
sakura_core/util/string_ex.cpp Outdated Show resolved Hide resolved
@berryzplus berryzplus marked this pull request as draft February 26, 2022 04:47
@AppVeyorBot
Copy link

@berryzplus berryzplus marked this pull request as ready for review February 26, 2022 11:27
sanomari
sanomari previously approved these changes Feb 27, 2022
Copy link
Contributor

@sanomari sanomari left a comment

Choose a reason for hiding this comment

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

変更してもらってよいです。

sakura_core/util/string_ex.cpp Outdated Show resolved Hide resolved
sakura_core/util/string_ex.cpp Outdated Show resolved Hide resolved
sakura_core/util/string_ex.cpp Outdated Show resolved Hide resolved
@berryzplus berryzplus marked this pull request as draft February 27, 2022 06:47
@sonarcloud
Copy link

sonarcloud bot commented Feb 27, 2022

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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

@berryzplus berryzplus marked this pull request as ready for review February 27, 2022 10:16
@berryzplus berryzplus merged commit 3b90cb2 into sakura-editor:master Feb 27, 2022
@berryzplus berryzplus deleted the feature/remake_strprintf branch February 27, 2022 10:20
@berryzplus
Copy link
Contributor Author

approveありがとうございます。
mergeしちゃいますので、何かあれば別PRで訂正・改善をお願いします。

@berryzplus
Copy link
Contributor Author

👆コメントしたつもりが入ってなかったので、後追いで入れました。

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