include tchar.h 削除

[gorogoro123]

PR対象

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

カテゴリ

• 改善

PR の背景

Unicode版のみ対応だが、tchar.h をincludeしている。
これまでの対応が不十分で、tchar.h を使用する関数が残っていたため、
#2066
で削除した。

仕様・動作説明

1. _T() マクロ(tchar.h) を TEXT() マクロ (winnt.h) に置き換える。
2. include <tchar.h> を削除する。

PR の影響範囲

影響なし。

テスト内容

1. コンパイル設定で、プリプロセッサ - ファイル前の処理 - はい(/P) に設定、前処理済み出力をファイル(*.i)に書き込み、変更前後で同じであることを確認する。
   96af681
   Debug/Win32 build 時のプリプロセッサ処理結果

CShareData::InitShareData()
std::wstring strShareDataName = GSTR_SHAREDATA;
std::wstring strShareDataName = (L"SakuraShareData" L"" L"WP" L"_DEBUG" L"181");

CNormalProcess::_GetInitializeMutex()
std::wstring strMutexInitName = GSTR_MUTEX_SAKURA_INIT;
std::wstring strMutexInitName = (L"MutexSakuraEditorInit" L"" L"WP" L"_DEBUG" L"181");

2. 変更前後でasmが同じことを確認する。
   96af681

関連 issue, PR

#2066

参考資料

https://learn.microsoft.com/ja-jp/windows/win32/api/winnt/nf-winnt-text

[berryzplus]

全体的に「しなくてよい変更」に見えます。

[berryzplus]

置換より削除がベターと思います。
プリコンパイル済みヘッダーでまとめてインクルードするように変えているため。

Suggested change

#include <wchar.h>

[berryzplus]

主題と無関係と思います。

似たようなのたくさんあるけど、横並びでNG。

[berryzplus]

この部分について、
「常にワイド文字列として使われる定数をナロー文字列で宣言するのはおかしい」
の理屈は理解できます。

純粋に「変える必要あるの？ないの？」で考えたときに「ない側に振れるんじゃないか」と言ってます。

[berryzplus]

_T() よりも TEXT() のがわかりやすい、は個人の主観であるように思います。

他ファイルでは tchar.h が不要になった。←了解。

tchar.h で定義していたマクロが使えなくなったので代替マクロを使う　←なんで？

[berryzplus]

_T()をTEXT()に置き換える理由がありません。

• 置き替えたい理由
• 置き換えたほうがよい理由
• 置き替えないといけない理由

特に理由がなければ、TEXT()よりも_T()のほうが短いため、「可能であれば短い記述が好ましい」の一般論に従って reject する感じになります。

他の部分で tchar.h の include を削除するのは構いませんが、
_T() を使うファイルでは （必要なので） tchar.h の include を残す、になると思います。

[sonarqubecloud]

Quality Gate failed

Failed conditions
40.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[berryzplus]

レビュー結果更新します。

• 不要になっている tchar.h のインクルードを削除。　←同意できます。
• 文字列定数の定義に _T() の代わりに ワイドサフィックス を使う。　←同意できません。

Pull Requestを分割したら論点がより明確になるのではないかと思います。