ファイル名変更通知ブロードキャストの頻度を抑制 #2162
ファイル名変更通知ブロードキャストの頻度を抑制 #2162
Conversation
beru
added
refactoring
| wcsncpy_s(filePath, _countof2(filePath), pszFilePath, _TRUNCATE ); | ||
| if (wcscmp(filePath, p->m_szFilePath) != 0) { | ||
| p->m_szFilePath = filePath; | ||
| changed = true; |
There was a problem hiding this comment.
指摘じゃないです。
ここのコードは結合テストでないとテストしづらいので、if文の追加は極力避けるべきと思います。
changed = 0 != wcscmp(変更前文字列, 変更後文字列);
ストア先変数 = 変更後文字列;
のように書けば、事前コピーとif文が要らないんじゃないかと思いました。
There was a problem hiding this comment.
コピー元の文字列 (pszFilePath) が極端に長い時にコピー先に切り詰めて記録されるので、コピー元の文字列が以前と変わってたとしてもコピー先の文字列の内容が以前と変わらない場合があるのではないかと思います。
ただコピー元の文字列が極端に長い場合があり得るのかの検証はしていません。
There was a problem hiding this comment.
もう少し詰めるとこういうことです。
changed |= 0 != _wcsncmp(変更後文字列, ストア先変数, std::size(ストア先変数) - 1);
wcsncpy_s(ストア先変数, std::size(ストア先変数), 変更後文字列, _TRUNCATE);C++20以降なら「変更後文字列.starts_with(ストア先変数)」と書けます。
ストア先変数のサイズはかなり大きくてMAXはタイトルバーに入りきらない気がするので、あまり気にしなくてよいかも知れません。（要検証。）
There was a problem hiding this comment.
バッファサイズについて少し見てみました。なんか統一されてないですね。
CEditWnd::ChangeFileNameNotify 呼び出し元の CEditWnd::UpdateCaption でローカル変数で用意しているキャプションのバッファサイズは1024文字分 link
CEditWnd::ChangeFileNameNotify で設定するタブのキャプション書き込み先（EditNode::m_szTabCaption）のバッファサイズは260文字分 link
CEditWnd::DispatchEvent で MYWM_TAB_WINDOW_NOTIFY メッセージの処理で CTabWnd::TabWindowNotify を呼び出して case TWNT_FILE の処理で用意される文字列バッファのサイズは1024文字分 link
ストア先変数のサイズはかなり大きくてMAXはタイトルバーに入りきらない気がするので、あまり気にしなくてよいかも知れません。（要検証。）
いやしかしberryzplusさんが将来的にLas Vegas Sphereを買い取ってサクラエディタの画面を映したとしたらタブの見出しに長い文字列を表示できる可能性も？
| if (changed) { | ||
| //ファイル名変更通知をブロードキャストする。 | ||
| int nGroup = CAppNodeManager::getInstance()->GetEditNode( GetHwnd() )->GetGroup(); | ||
| CAppNodeGroupHandle(nGroup).PostMessageToAllEditors( |
There was a problem hiding this comment.
指摘じゃないんですが。
これ
int nGroup = CAppNodeManager::getInstance()->GetEditNode( GetHwnd() )->GetGroup();
CAppNodeGroupHandle(nGroup).PostMessageToAllEditors(
は
CAppNodeManager::getInstance()->GetEditNode( GetHwnd() )->GetGroup().PostMessageToAllEditors(
と等価なので、冗長な書き方だと思います。
There was a problem hiding this comment.
確かにそうですね。ただこのPRでは変更しないようにします。
There was a problem hiding this comment.
「他にもたくさんある」というのと、こういうのとどちらがよいか、みたいな話もあります。
auto& cGroup = CAppNodeManager::getInstance()->GetEditNode( GetHwnd() )->GetGroup();
cGroup.PostMessageToAllEditors(
2 participants
PR対象
カテゴリ
PR の背景
#2153 (comment) に記載
タイトルが更新されない場合にも
CEditWnd::ChangeFileNameNotifyの最後でファイル名変更通知をブロードキャストしてCEditWnd::DispatchEventのcase MYWM_TAB_WINDOW_NOTIFYで処理するのが定期的に実行されているのでもったいないなと思いました。仕様・動作説明
タブの見出しやファイル名の内容が変わっていない場合はファイル名変更通知をブロードキャストしないようにしました。
PR の影響範囲
テスト内容
タブ名が今まで通りと変わらず変更される事を確認
関連 issue, PR
参考資料