-
Notifications
You must be signed in to change notification settings - Fork 168
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
単体テストで文字列リソースを利用できるようにする #1275
単体テストで文字列リソースを利用できるようにする #1275
Conversation
英語版リソースも同様にテストするのは可能ですか? |
テストコード側で実際のリソースを読めないと困る事ってあるんでしょうか? |
なんでやねん!w と思ったことを正直に書いておきます。 |
✅ Build sakura 1.0.2783 completed (commit 9d998f4885 by @berryzplus) |
言語切替は
|
たとえば sakura/sakura_core/CSelectLang.cpp Lines 101 to 109 in e0ba361
102行目と108行目のassertです。 このPRで入れようとしてるテスト対象リソースが読めなかったら落ちるようになっています。 ということで、「これ入れないと |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
動作確認しました。問題ないと思います。
@@ -101,6 +101,7 @@ | |||
</ItemDefinitionGroup> | |||
<ItemGroup> | |||
<Link Include="$(SolutionDir)sakura\$(Platform)\$(Configuration)\*.obj" /> | |||
<Link Include="$(SolutionDir)sakura\$(Platform)\$(Configuration)\*.res" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これだけでリソースを組み込むことが出来るんですね。目から鱗です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rcをコンパイルしたファイルの拡張子が.obj
じゃなかったので組み込まれてなかった感じです。実は、MinGW版のtests1.exeにはかなり前から組み込まれていました。
{ | ||
// リソースから言語識別子のIDを読み取る | ||
// ここのリソース文字列値は、言語選択のキーなので変更してはならない。 | ||
ASSERT_STREQ( L"0x0411", LS( STR_SELLANG_LANGID ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このPRの変更内容には関係ないコメントです。
LS
マクロは CLoadString::LoadStringSt
の呼び出しになるのでそこからの処理を追ってみました。CLoadString::CLoadStrBuffer::LoadString
を見ると色々とやっているんですね。単にWindowsAPIを呼び出しているだけかと思ってました…。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
裏側がかなり高度なリサイクルバッファになっていたはず。
リソース文字列はUTF16LEのWORDシーケンスとして格納されるので、本来的にバッファを用意してコピーする必要はないんですが、NUL終端文字列として扱えるようにするために再利用可能なバッファの仕組みが組み込んである感じです。
他のリソース種類と異なり、RT_STRINGは単なるデータの塊なので、std::wstring_view
で直接参照してしまえばバッファ問題は解決できるんじゃないかとか、何気にやや危険なことを前々から思っていたりします。。。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::basic_string_view
って NULL終端してなくても良いんですね。部分を参照するのに使われる事があるからそういう仕様なんですね。
https://stackoverflow.com/a/41722032/4699324
ただ format 系の処理で今使っているのは printf 系の関数なのでそちらがNULL終端に対応している事を要求しますね。言語的にNULL終端の流れは切れなそう…。
なおリソース文字列の長さを取るには LoadStringW
を使わないやり方が必要なようです。
https://devblogs.microsoft.com/oldnewthing/20040130-00/?p=40813
リソース文字列の容量ってそんな大きくないだろうから起動時に全部メモリに入れてしまった方が使う度にいちいちメモリ確保してコピーとか避けられるんじゃとか思いますが、実現しようと取り掛かってやっと分かる事も多そうです。
stub を用意する必要があるかと思ったらそんな事は無くて簡単な変更でリソースを組み込めたんですね。本物の実装をそのまま使えるならそれがベストですね。 |
レビューありがとうございます。 |
…_resource_to_vctest 単体テストで文字列リソースを利用できるようにする
(必須) PR の目的
単体テストで文字列リソースを利用できるようにします。
(必須) カテゴリ
(自明なら省略可) PR の背景
#1273 (comment)
(自明なら省略可) PR のメリット
(なければ省略可) PR のデメリット (トレードオフとかあれば)
(わかる範囲で) PR の影響範囲
(なければ省略可) 関連 issue, PR
(なければ省略可) 参考資料