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

リソースファイルのリファクタリング (複数行にわかれているのを一行にまとめる)の対応漏れの対応 #320

Merged
merged 2 commits into from
Aug 5, 2018

Conversation

m-tmatma
Copy link
Member

@m-tmatma m-tmatma commented Aug 5, 2018

リソースファイルのリファクタリング (複数行にわかれているのを一行にまとめる) #318
の対応漏れの対応

対応方法

  • Visual Studio 2017 を使って置換を行う
  • 正規表現を有効にする
  • *.rc ファイル を対象にする

置換パターン

変換前: "\bNOT\s*\r\n\s*"
変換後: "NOT "

(↑ 変換後のパターンで末尾に空白を含む。ダブルクオートは含まない)
→ 24 箇所置換した。

確認方法

PR の最新のコミットを取得

以下コマンドでビルド

build-sln.bat Win32 Release
build-sln.bat Win32 Debug
build-sln.bat x64   Release
build-sln.bat x64   Debug

以下コマンドで移動

mkdir sakura_lang_en_US\New\
mkdir sakura\New\
move sakura_lang_en_US\Win32 sakura_lang_en_US\New\
move sakura_lang_en_US\x64   sakura_lang_en_US\New\
move sakura\x64              sakura\New\
move sakura\Win32            sakura\New\

分岐元の git hash を表示

git show-branch --merge-base

分岐元の git hash を取得

git checkout 671f7dc0baa28edde8226f10dc6eaca5e61c6bdf

githash.bat の先頭に exit /b 0 を挿入して githash.h が更新されないようにする。

以下コマンドでビルドする

build-sln.bat Win32 Release
build-sln.bat Win32 Debug
build-sln.bat x64   Release
build-sln.bat x64   Debug

以下コマンドで移動

mkdir sakura_lang_en_US\Old\
mkdir sakura\Old\
move sakura_lang_en_US\Win32 sakura_lang_en_US\Old\
move sakura_lang_en_US\x64   sakura_lang_en_US\Old\
move sakura\x64              sakura\Old\
move sakura\Win32            sakura\Old\

#317 のスクリプトを実行する

calc-hash-res.py hash.txt .

以下ローカルビルドの結果です。

hash.txt

- Visual Studio 2017 を使って置換を行う
- 正規表現を有効にする
- *.rc ファイル を対象にする

置換パターン
  変換前: "\bNOT\s*\r\n\s*"
  変換後: "NOT "

(↑ 変換後のパターンで末尾に空白を含む。ダブルクオートは含まない)
→ 24 箇所置換した。
@m-tmatma m-tmatma added the refactoring リファクタリング 【ChangeLog除外】 label Aug 5, 2018
@m-tmatma m-tmatma added this to the next release milestone Aug 5, 2018
@berryzplus
Copy link
Contributor

さすがです。まったく気づいておりませんでした(マテ

この対応はしておいたほうが良いと思っています。
(漏れなく変更できているかチェックする対応、ですね)

で、困ったことがあります。

じゃぁ、本当にもれなく変更できているか確認できる条件はなんだ?と考えて、
^[\t ]+.+ で *.rc ファイルを検索した結果を眺めてみたんです。(一致行6065です)

こういうの見つけました。

  sakura_lang_en_US\sakura_lang_rc.rc(1217):
    CONTROL         "Enable CR Code NEL,PS,LS(&O)",IDC_CHECK_ENABLEEXTEOL,"Button",BS_AUTOCHECKBOX | WS_GROUP
                   | WS_TABSTOP,161,138,122,10

手書きしてますね、これは・・・。
フラグのOR演算子が行頭に来ているから漏れたパターンです。

修正漏れがないかの確認は、目視でやるしかない気がしています。
他にないかはもうちょっと見ときます。

@berryzplus
Copy link
Contributor

OK、日英とも1箇所だけでした。
行番号は f4d3f4c のものです。
対応をお願いします。

  C:\gitroot\sakura\sakura_core\sakura_rc.rc(1215):    CONTROL         "改行コードNEL,PS,LSを有効にする(&O)",IDC_CHECK_ENABLEEXTEOL,"Button",BS_AUTOCHECKBOX | WS_GROUP
  C:\gitroot\sakura\sakura_core\sakura_rc.rc(1216):                    | WS_TABSTOP,161,138,122,10
  C:\gitroot\sakura\sakura_lang_en_US\sakura_lang_rc.rc(1217):    CONTROL         "Enable CR Code NEL,PS,LS(&O)",IDC_CHECK_ENABLEEXTEOL,"Button",BS_AUTOCHECKBOX | WS_GROUP
  C:\gitroot\sakura\sakura_lang_en_US\sakura_lang_rc.rc(1218):                    | WS_TABSTOP,161,138,122,10

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.

対応ありがとうございます。
LGTMです。

@berryzplus
Copy link
Contributor

せっかくなのでこれもやりませんか?

カンマの後の空白を1個に統一
s/((?<!\\)"(?:[^\\"]|\\[^\\])*"|\w+),(?: {2,}|\t+)?(\w+|"(?:[^\\"]|\\[^\\])*")/$1, $2/

@m-tmatma
Copy link
Member Author

m-tmatma commented Aug 5, 2018

カンマの後の空白を1個に統一
s/((?<!\)"(?:[^\\"]|\[^\\])"|\w+),(?: {2,}|\t+)?(\w+|"(?:[^\\"]|\[^\\])")/$1, $2/

難しい正規表現ですね。
ぱっと正当性を確認できないです。

@berryzplus
Copy link
Contributor

カンマの後の空白を1個に統一
s/((?<!\\)"(?:[^\\"]|\\[^\\])*"|\w+),(?: {2,}|\t+)?(\w+|"(?:[^\\"]|\\[^\\])*")/$1, $2/

単純化すると s/,/, / です。
カンマの後に空白を1つだけ付けたい、という置換。
パターンだけ見たらぼくも判断は難しいと思います。
やるなら少し説明がいると思ってます。

普通にやると引用符に囲まれたカンマまで処理してしまうので前後を付けています。

  1. ((?<!\\)"(?:[^\\"]|\\[^\\])*"|\w+) 先行パート、キャプチャします。
    先行パートの構成物は 文字列か識別子 です。
    否定戻り読み(?<!...)でエスケープされてない二重引用符を探してます。
    文字列の中身は0個以上の文字ですがエスケープシーケンスは除外したいので複雑になってます。
  2. ,(?: {2,}|\t+)? 置換対象パート、キャプチャしません。
    カンマの後に2個以上の半角空白か1個以上のタブ文字があった場合はここにマッチします。
  3. (\w+|"(?:[^\\"]|\\[^\\])*") 後続パート、キャプチャします。
    後続パートの構成物は 識別子か文字列 です。
    パフォーマンスを考慮して識別子を先に判定させます。
    先行文字列が決まっているので戻り読みは不要です。

他に考慮すべきケースがあるかどうかはチェックしてますが、
たぶんこれでも抜けが出るんじゃないかと思っています。
この正規表現で保証できるのは文字列中のカンマを誤って置換しないことなので、
すべてのカンマの後ろに空白がついたかどうかを別にチェックしないといかんです。

@m-tmatma
Copy link
Member Author

m-tmatma commented Aug 5, 2018

単純化すると s/,/, / です。
カンマの後に空白を1つだけ付けたい、という置換。

はい。

パターンだけ見たらぼくも判断は難しいと思います

他に考慮すべきケースがあるかどうかはチェックしてますが、
たぶんこれでも抜けが出るんじゃないかと思っています。

この PR ではやらずに別の PR にするのがいいと思います。
@berryzplus さん PR お願いします。

この PR は一旦マージします。

@m-tmatma m-tmatma merged commit db3c108 into sakura-editor:master Aug 5, 2018
@m-tmatma m-tmatma deleted the concat-res-trailing-not branch August 5, 2018 10:04
@ds14050 ds14050 added the refactoring リファクタリング 【ChangeLog除外】 label Sep 18, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…g-not

リソースファイルのリファクタリング (複数行にわかれているのを一行にまとめる)の対応漏れの対応
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants