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

git情報が存在しないとき、バージョンダイアログにバージョン番号が表示されない&文字化けする問題を修正 #150

Merged
merged 2 commits into from
Jun 21, 2018

Conversation

kobake
Copy link
Member

@kobake kobake commented Jun 20, 2018

git 情報が無い状態でビルドしたとき(git clone ではなく zip で入手したとき等)、バージョン情報ダイアログにバージョン番号が表示されない&文字化け(というか未初期化バッファが展開されてしまう)が発生する問題を修正しました。

文字化け症状について

@arigayas さんからご報告いただいてました。

https://twitter.com/arigayas/status/1009446814041305093

マージされたっぽいので早速ビルドしてみました。謎の文字にビックリしましたがビルド出来ましたww
ari

修正後:git情報が有るときのバージョンダイアログの見た目

with-git

修正後:git情報が無いときのバージョンダイアログの見た目

without-git

さらにいうと szMsg 未初期化により実は文字化けすることがありました。それも副産物的に修正。
@kobake
Copy link
Member Author

kobake commented Jun 20, 2018

https://twitter.com/arigayas/status/1009443578068627458
「情報をコピー」のところも追加対応必要だったかもです。明日見ます。

@kobake
Copy link
Member Author

kobake commented Jun 20, 2018

https://twitter.com/arigayas/status/1009443578068627458
「情報をコピー」のところも追加対応必要だったかもです。明日見ます。

今確認しましたが、「情報をコピー」のところで用いられる情報は
本PRで既に修正していた箇所をもとに生成された情報だったので
こちらも問題なく修正されていることを確認しました。
ですのでここについての追加対応は必要無しです。

@kobake kobake changed the title git情報が存在しないとき、バージョンダイアログにバージョン番号が表示されない問題を修正 git情報が存在しないとき、バージョンダイアログにバージョン番号が表示されない&文字化けする問題を修正 Jun 20, 2018
@m-tmatma m-tmatma added the 🐛bug🦋 ■バグ修正(Something isn't working) label Jun 20, 2018
@m-tmatma m-tmatma added this to the next release milestone Jun 20, 2018
@kobake kobake closed this Jun 20, 2018
@kobake kobake reopened this Jun 20, 2018
m-tmatma
m-tmatma previously approved these changes Jun 20, 2018
Copy link
Member

@m-tmatma m-tmatma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appveyor をリビルドしておいてください。

@kobake
Copy link
Member Author

kobake commented Jun 20, 2018

06:16 の close/reopen 操作でリビルドを走らせています(このやり方ちょっとダサい気がするけどいつもこれでリビルド走らせています)

@m-tmatma
Copy link
Member

またダメみたいですね

@kobake
Copy link
Member Author

kobake commented Jun 20, 2018

('A`)

@kobake kobake closed this Jun 20, 2018
@kobake kobake reopened this Jun 20, 2018
@kobake
Copy link
Member Author

kobake commented Jun 20, 2018

また止まってる……もしかしたらたまたまサービス側が不安定な時間帯であることも考えられるので、もうすこし時間おいてから再ビルド走らせてみます

@k-takata
Copy link
Member

close/reopen 操作でリビルドを走らせています

AppVeyor上で "RE-BUILD COMMIT" ボタンを押せばいいと思います。
目的外のclose操作は、見ている人が混乱します。

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です。

詳細にこだわるかどうかは別の話と考えとります。

HIWORD(dwVersionMS),
LOWORD(dwVersionMS),
HIWORD(dwVersionLS),
LOWORD(dwVersionLS)
);
cmemMsg.AppendString(szMsg);
#if defined(GIT_COMMIT_HASH)
auto_sprintf(szMsg, _T("(GitHash " GIT_COMMIT_HASH ")\r\n"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wcscpyで良くないですか?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sakura-editor/management-forum#4 これの結論次第ではそれもあり得ますが、今は現状のコーディングスタイルを踏襲してます。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここ、実はそのままappendしてもいいですよね〜

別件扱い了解です。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto_sprintf で整形しなくても
直で cmemMsg.AppendString を呼ぶのでも
いいかも。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントかぶった

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そのままappendはたしかに。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接append方式に変更しました。

@kobake
Copy link
Member Author

kobake commented Jun 21, 2018

AppVeyor上で "RE-BUILD COMMIT" ボタンを押せばいいと思います。
目的外のclose操作は、見ている人が混乱します。

ありがとうございます。RE-BUILD 操作でいけました。

「NEW BUILD」「RE-BUILD PR」の2つのボタンがあり、前に試したときには右側のボタンの存在に気づいてなくて「NEW BUILD」のほうを押したら対象PRではなく master ビルドの再実行になってしまっいたのでアレ?と思い仕方なく close/reopen の操作をしていた、、という経緯です(´Д`)

@kobake
Copy link
Member Author

kobake commented Jun 21, 2018

すみません、今スマホでappveyor操作できないのでclose/reopenで再ビルド走らせます

@kobake kobake closed this Jun 21, 2018
@kobake kobake reopened this Jun 21, 2018
@kobake
Copy link
Member Author

kobake commented Jun 21, 2018

やっとビルド通りました(´Д`)
問題なさそうであれば approve && merge お願いします

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 berryzplus merged commit 53b1d94 into sakura-editor:master Jun 21, 2018
@kobake kobake deleted the version-dialog-without-githash branch June 21, 2018 11:32
@m-tmatma m-tmatma added the twitter twitter label Jun 21, 2018
@ds14050 ds14050 added the 🐛bug🦋 ■バグ修正(Something isn't working) label Sep 18, 2018
@ds14050 ds14050 added the twitter twitter label Sep 18, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…ut-githash

git情報が存在しないとき、バージョンダイアログにバージョン番号が表示されない&文字化けする問題を修正

マージしちゃいまーす。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working) twitter twitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants