バージョン情報ダイアログの整理 #569

berryzplus · 2018-10-20T09:45:01Z

バージョン情報ダイアログのコード（＋リソース）の整理を行います。

#563 #566 と関係する話題です。

バージョン情報ダイアログには、ビルド時のソースコード情報が表示されます。
これらの情報は、実行時に動的に取得する種類の情報ではありませんが、
現在のソースコードでは、実行時ダイアログを表示する直前に情報を書き換えています。

このコミットでは、シンボルの定義状態に基づいた表示切替を反映した形でダイアログリソースを構築します。変更により、ダイアログ初期化時のコードの大半が消えることになります。

#566 の背景色描画範囲がおかしい問題も同時に解決される見込みです。

このダイアログリソースには、まだいろいろと課題がありますが、
今回対応するのはとりあえずリンクアドレスの埋め込みと表示切替の反映のみです。

表示非表示をコードで切り替えていたのを変更。ビルド元ソースの情報は、実行時に変わることはないので、埋め込んでしまう。

m-tmatma · 2018-10-20T10:39:35Z

sakura_core/sakura_rc.rc

+    LTEXT           APPVEYOR_SHORTHASH, IDC_STATIC_URL_GITHUB_COMMIT, 101, 133, 30, 8, SS_NOTIFY | NOT WS_GROUP | WS_TABSTOP
+#endif
+#ifdef APPVEYOR_PULL_REQUEST_NUMBER
+    LTEXT           "PR " APPVEYOR_PULL_REQUEST_NUMBER, IDC_STATIC_URL_GITHUB_PR, 150, 133, 30, 8, SS_NOTIFY | NOT WS_GROUP | WS_TABSTOP


APPVEYOR_PULL_REQUEST_NUMBER のところで
リソースファイルでは c/c++ のように文字列連結はできないはずです。

また、appveyor の過去の PR の artifacts から Log の zip をダウンロードして
set_appveyor_env.bat をダウンロードして、コマンドプロンプトから実行して
ビルドすれば PR でのビルドをエミュレートできるので、ローカルで PR 固有の
ビルドのテストをできます。

APPVEYOR_PULL_REQUEST_NUMBER のところで
リソースファイルでは c/c++ のように文字列連結はできないはずです。

やってみてできなかったので別な方法考えます。

また、appveyor の過去の PR の artifacts から Log の zip をダウンロードして
set_appveyor_env.bat をダウンロードして、コマンドプロンプトから実行して
ビルドすれば PR でのビルドをエミュレートできるので、ローカルで PR 固有の
ビルドのテストをできます。

この文章が理解できてるか怪しいです。

githash のシンボル群は git を使って生成してるから zip 版だと hash の部分が真っ白になる認識でした。zipでも追加作業したらHash ID出せるといってます？

この文章が理解できてるか怪しいです。

例えば https://ci.appveyor.com/project/sakuraeditor/sakura/builds/19644986 にアクセスして
適用な構成のビルドの artifacts から Log のついている ZIP をダウンロードします。

ダウンロードした ZIP を解凍すると中に set_appveyor_env.bat が入っています。

set_appveyor_env.bat で appveyor でビルドしたときと同じ状態で環境変数が定義されているので、取得した set_appveyor_env.bat をコマンドプロンプトで実行します。

build-sln.bat でビルドします。

→ appveyor でビルドしたのと同等の環境でビルドしたのを再現できます。
(必要に応じてローカルで set_appveyor_env.bat を編集して試してもいいと思います)
(注意点としては成功したビルドしか set_appveyor_env.bat は収集されないので
今試している PR の artifacts を取得する場合は appveyor 上で少なくとも一度ビルドを
通す必要があります)

set_appveyor_env.bat の例

set APPVEYOR_BUILD_FOLDER=C:\projects\sakura set APPVEYOR_REPO_PROVIDER=gitHub set APPVEYOR_REPO_NAME=sakura-editor/sakura set APPVEYOR_REPO_COMMIT=0fd870a632a26664ad4a1d601b5c2a4a4361a251 set APPVEYOR_REPO_TAG_NAME= set APPVEYOR_PULL_REQUEST_NUMBER=565 set APPVEYOR_PULL_REQUEST_HEAD_COMMIT=b5c86256710d42cf054c834aa0eeca1b250da77a set APPVEYOR_URL=https://ci.appveyor.com set APPVEYOR_ACCOUNT_NAME=sakuraeditor set APPVEYOR_PROJECT_SLUG=sakura set APPVEYOR_BUILD_VERSION=1.0.1050 set APPVEYOR_BUILD_NUMBER=1050

m-tmatma · 2018-10-20T13:11:40Z

appveyor.md

@@ -164,6 +164,7 @@
 | APPVEYOR_BUILD_NUMBER_INT         | APPVEYOR_BUILD_NUMBER       |数値     |
 | APPVEYOR_PULL_REQUEST_NUMBER      |APPVEYOR_PULL_REQUEST_NUMBER |文字列   |
 | APPVEYOR_PULL_REQUEST_NUMBER_INT  |APPVEYOR_PULL_REQUEST_NUMBER |数値     |
+| APPVEYOR_PULL_REQUEST_NUMBER_LABEL  |APPVEYOR_PULL_REQUEST_NUMBER |文字列   |


細かいですが、
APPVEYOR_PULL_REQUEST_NUMBER の前の | で列を前後の行と揃えていると見やすいです。

m-tmatma

master

この PR (build URL が消えている)

m-tmatma · 2018-10-20T13:23:40Z

APPVEYOR_BUILD_TEXTは CDlgAbout.cpp の中で定義されているので、
APPVEYOR_PULL_REQUEST_NUMBER_LABEL と同様に githash.h に生成するように
変更する必要があります。

sakura_core/dlg/CDlgAbout.cpp:#define APPVEYOR_BUILD_TEXT       "Build " APPVEYOR_BUILD_NUMBER
sakura_core/dlg/CDlgAbout.cpp:#if defined(APPVEYOR_BUILD_TEXT)
sakura_core/dlg/CDlgAbout.cpp:#pragma message("APPVEYOR_BUILD_TEXT: " APPVEYOR_BUILD_TEXT)
sakura_core/dlg/CDlgAbout.cpp:#ifdef APPVEYOR_BUILD_TEXT
sakura_core/sakura_rc.rc:#ifdef APPVEYOR_BUILD_TEXT
sakura_core/sakura_rc.rc:    LTEXT           APPVEYOR_BUILD_TEXT, IDC_STATIC_URL_APPVEYOR_BUILD, 101, 123, 120, 8, SS_NOTIFY | NOT WS_GROUP | WS_TABSTOP
sakura_lang_en_US/sakura_lang_rc.rc:#ifdef APPVEYOR_BUILD_TEXT
sakura_lang_en_US/sakura_lang_rc.rc:    LTEXT           APPVEYOR_BUILD_TEXT, IDC_STATIC_URL_APPVEYOR_BUILD, 101, 123, 120, 8, SS_NOTIFY | NOT WS_GROUP | WS_TABSTOP

m-tmatma · 2018-10-20T13:25:45Z

↑ もともとのコードがわかりにくいコードになってました。

sakura/githash.bat

m-tmatma · 2018-10-20T20:52:50Z

appveyor.md

-| APPVEYOR_SHORTHASH_PR_HEAD        |APPVEYOR_SHORTHASH_PR_HEAD   |文字列   |
-| APPVEYOR_BUILD_URL                |APPVEYOR_BUILD_URL           |文字列   |
+| 生成するマクロ名                | 元にする環境変数                | 型
+| ----                            | ----                            | ----


右端の | は残しておきたいです。

理由は
sakura-editor/management-forum#31 (comment)
のプラグインでは | をつけるからです。

m-tmatma · 2018-10-21T03:52:02Z

build URL が消えている件、お願いします。

m-tmatma · 2018-10-21T07:02:09Z

sakura_lang_en_US/sakura_lang_rc.rc

+    LTEXT           GIT_URL, IDC_STATIC_URL_GIT, 101, 113, 120, 8, SS_NOTIFY | NOT WS_GROUP | WS_TABSTOP
+#endif
+#ifdef APPVEYOR_BUILD_LABEL
+	LTEXT           "Build URL:", IDC_STATIC_URL_APPVEYOR_CAPTION, 33, 123, 71, 8, NOT WS_GROUP


sakura_lang_en_US\sakura_lang_rc.rc と sakura_core\sakura_rc.rc とで
Build URL: の行と APPVEYOR_SHORTHASH の行とで空白の差分が出ています。

もともとは差分がなかったところなので差分がない(不要な変更がない)ように
してほしいです。

appveyor.md

表示非表示をコードで切り替えていたのを変更。ビルド元ソースの情報は、実行時に変わることはないので埋め込んでしまう。

リソーススクリプト内でリテラル文字列の連結が行われない対策として、 githash.batでラベル用のリテラル文字列を生成させるように変更。当初cpp上で定義されていた既存変数に当て込もうとしていたが、ビルド番号の文字列表現と取り違える危険があるので名称変更。 APPVEYOR_BUILD_TEXT → APPVEYOR_BUILD_NUMBER_LABEL BUILD_NUMBER_LABELは"Build: xxxx"を定義したシンボル。

リソーススクリプト内でリテラル文字列の連結が行われない対策として、 githash.batでラベル用のリテラル文字列を生成させるように変更。当初APPVEYOR_PULL_REQUEST_NUMBER_LABELで作成したが、シンボル名が長すぎて切り詰められるので短い名前に変更。切り詰めが発生したときのログ↓ 3>..\sakura_core\sakura_rc.rc(119): warning RC4011: identifier truncated to 'APPVEYOR_PULL_REQUEST_NUMBER_LA' 3> 3>sakura.vcxproj -> F:\gitroot\sakura-mygithub\Win32\Debug\sakura.exe

インデントがずれていたのを修正。 markdown表の末尾縦棒はExcelアドインの出力に合わせ残す。 sakura-editor/management-forum#31 (comment)

berryzplus · 2018-10-21T08:55:46Z

不要な変更を消すためにリベースしました。
対象コミットが先頭のほうにあったので、
ついでで後半のコミットを組みなおしました。

指摘内容はすべて取り込んだつもり・・・ 😢

m-tmatma · 2018-10-21T09:24:11Z

sakura_core/sakura_rc.rc

-    LTEXT           "Commit", IDC_STATIC_URL_GITHUB_COMMIT, 101, 133, 30, 8, SS_NOTIFY | NOT WS_GROUP | WS_TABSTOP
-    LTEXT           "PR.", IDC_STATIC_URL_GITHUB_PR, 150, 133, 30, 8, SS_NOTIFY | NOT WS_GROUP | WS_TABSTOP
+#endif
+#ifdef APPVEYOR_SHORTHASH


sakura/sakura_core/dlg/CDlgAbout.cpp

Lines 321 to 339 in f753d55

case IDC_STATIC_URL_APPVEYOR_BUILD:

{

#if defined(APPVEYOR_BUILD_URL)

::ShellExecute(GetHwnd(), NULL, _T(APPVEYOR_BUILD_URL), NULL, NULL, SW_SHOWNORMAL);

#elif defined(GIT_URL)

::ShellExecute(GetHwnd(), NULL, _T(GIT_URL), NULL, NULL, SW_SHOWNORMAL);

#endif

return TRUE;

}

case IDC_STATIC_URL_GITHUB_COMMIT:

#if defined(GITHUB_COMMIT_URL)

::ShellExecute(GetHwnd(), NULL, _T(GITHUB_COMMIT_URL), NULL, NULL, SW_SHOWNORMAL);

#endif

return TRUE;

case IDC_STATIC_URL_GITHUB_PR:

#if defined(GITHUB_COMMIT_URL_PR_HEAD)

::ShellExecute(GetHwnd(), NULL, _T(GITHUB_COMMIT_URL_PR_HEAD), NULL, NULL, SW_SHOWNORMAL);

#endif

return TRUE;

厳密に言えば、APPVEYOR_BUILD_URL や GITHUB_COMMIT_URL や GITHUB_COMMIT_URL_PR_HEAD などにも依存しているのでこれら
のマクロの定義もチェックした方がいいとは思います。

ただこの PR で全部やらなくてもいいとは思います。

これらのマクロを全部チェックすると複雑になるので各コントロールが
有効になるか無効になるのかをマクロを別途定義して、切り替えるように
したら簡単なんじゃ開花と思います。

厳密に言えば、APPVEYOR_BUILD_URL や GITHUB_COMMIT_URL や GITHUB_COMMIT_URL_PR_HEAD などにも依存しているのでこれら
のマクロの定義もチェックした方がいいとは思います。

ただこの PR で全部やらなくてもいいとは思います。

欲を言うとシンボルの定義状況によって処理が変わるのはあまりよくないんじゃないかと思っています。とりあえず、このPRの目標は、ビルド時に確定して変わらないはずの表示テキストと表示非表示をリソースに埋め込むことなので残課題は残す前提で進めたいです。

m-tmatma

対応ありがとうございました。

m-tmatma · 2018-10-21T09:56:06Z

berryzplus · 2018-10-21T10:16:32Z

あざーっす。マージしまーっす 😄

…ight_dlg_about バージョン情報ダイアログの整理

ダイアログリソース整理　GIT_URL

65036bd

表示非表示をコードで切り替えていたのを変更。ビルド元ソースの情報は、実行時に変わることはないので、埋め込んでしまう。

berryzplus mentioned this pull request Oct 20, 2018

URLリンクの改善 #566

Merged

m-tmatma reviewed Oct 20, 2018

View reviewed changes

m-tmatma requested changes Oct 20, 2018

View reviewed changes

m-tmatma reviewed Oct 20, 2018

View reviewed changes

sakura/githash.bat Outdated Show resolved Hide resolved

m-tmatma reviewed Oct 20, 2018

View reviewed changes

m-tmatma mentioned this pull request Oct 20, 2018

Git Commit Hash 関連の環境変数名の修正 #576

Closed

m-tmatma reviewed Oct 21, 2018

View reviewed changes

appveyor.md Show resolved Hide resolved

berryzplus added 7 commits October 21, 2018 16:24

ダイアログリソース整理　APPVEYOR_BUILD_TEXT

983f655

表示非表示をコードで切り替えていたのを変更。ビルド元ソースの情報は、実行時に変わることはないので埋め込んでしまう。

ダイアログリソース整理　APPVEYOR_SHORTHASH

f2c30b6

表示非表示をコードで切り替えていたのを変更。ビルド元ソースの情報は、実行時に変わることはないので埋め込んでしまう。

ダイアログリソース整理　APPVEYOR_PULL_REQUEST_NUMBER

48dbf1a

表示非表示をコードで切り替えていたのを変更。ビルド元ソースの情報は、実行時に変わることはないので埋め込んでしまう。

ダイアログリソース整理　IDC_STATIC_URL_GITHUB_CAPTION

0107213

表示非表示をコードで切り替えていたのを変更。ビルド元ソースの情報は、実行時に変わることはないので埋め込んでしまう。

appveyor.mdの整形

f753d55

インデントがずれていたのを修正。 markdown表の末尾縦棒はExcelアドインの出力に合わせ残す。 sakura-editor/management-forum#31 (comment)

berryzplus force-pushed the refactoring/straight_dlg_about branch from 867be53 to f753d55 Compare October 21, 2018 08:45

m-tmatma mentioned this pull request Oct 21, 2018

APPVEYOR_PULL_REQUEST_NUMBER_LABEL ではなく APPVEYOR_PR_NUMBER_LABEL の名前にした経緯を markdown に残す #577

Closed

m-tmatma reviewed Oct 21, 2018

View reviewed changes

m-tmatma approved these changes Oct 21, 2018

View reviewed changes

berryzplus merged commit 87c4215 into sakura-editor:master Oct 21, 2018

berryzplus deleted the refactoring/straight_dlg_about branch October 21, 2018 10:17

m-tmatma added the refactoring リファクタリング【ChangeLog除外】 label Oct 21, 2018

m-tmatma added this to the next release milestone Oct 21, 2018

m-tmatma added the CI appveyor など CI 関連【ChangeLog除外】 label Oct 21, 2018

HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019

Merge pull request sakura-editor#569 from berryzplus/refactoring/stra…

32e8844

…ight_dlg_about バージョン情報ダイアログの整理

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

バージョン情報ダイアログの整理 #569

バージョン情報ダイアログの整理 #569

berryzplus commented Oct 20, 2018

m-tmatma Oct 20, 2018

berryzplus Oct 20, 2018

m-tmatma Oct 20, 2018

m-tmatma Oct 20, 2018

m-tmatma left a comment

m-tmatma commented Oct 20, 2018 •

edited

Loading

m-tmatma commented Oct 20, 2018

m-tmatma Oct 20, 2018

m-tmatma commented Oct 21, 2018

m-tmatma Oct 21, 2018

berryzplus commented Oct 21, 2018

m-tmatma Oct 21, 2018

berryzplus Oct 21, 2018

m-tmatma left a comment

m-tmatma commented Oct 21, 2018

berryzplus commented Oct 21, 2018

	case IDC_STATIC_URL_APPVEYOR_BUILD:
	{
	#if defined(APPVEYOR_BUILD_URL)
	::ShellExecute(GetHwnd(), NULL, _T(APPVEYOR_BUILD_URL), NULL, NULL, SW_SHOWNORMAL);
	#elif defined(GIT_URL)
	::ShellExecute(GetHwnd(), NULL, _T(GIT_URL), NULL, NULL, SW_SHOWNORMAL);
	#endif
	return TRUE;
	}
	case IDC_STATIC_URL_GITHUB_COMMIT:
	#if defined(GITHUB_COMMIT_URL)
	::ShellExecute(GetHwnd(), NULL, _T(GITHUB_COMMIT_URL), NULL, NULL, SW_SHOWNORMAL);
	#endif
	return TRUE;
	case IDC_STATIC_URL_GITHUB_PR:
	#if defined(GITHUB_COMMIT_URL_PR_HEAD)
	::ShellExecute(GetHwnd(), NULL, _T(GITHUB_COMMIT_URL_PR_HEAD), NULL, NULL, SW_SHOWNORMAL);
	#endif
	return TRUE;

バージョン情報ダイアログの整理 #569

バージョン情報ダイアログの整理 #569

Conversation

berryzplus commented Oct 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-tmatma left a comment

Choose a reason for hiding this comment

master

この PR (build URL が消えている)

m-tmatma commented Oct 20, 2018 • edited Loading

m-tmatma commented Oct 20, 2018

Choose a reason for hiding this comment

m-tmatma commented Oct 21, 2018

Choose a reason for hiding this comment

berryzplus commented Oct 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-tmatma left a comment

Choose a reason for hiding this comment

m-tmatma commented Oct 21, 2018

berryzplus commented Oct 21, 2018

m-tmatma commented Oct 20, 2018 •

edited

Loading