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

MakefileMake 出力メッセージを英語に変更 #44

Merged
merged 1 commit into from
Jun 3, 2018

Conversation

kobake
Copy link
Member

@kobake kobake commented Jun 2, 2018

概要

MakefileMake を日本語OS以外の環境で動かしてもメッセージを読めるように、出力メッセージを英語形式に変更。

変更前後の挙動

  • 変更前: SJIS日本語出力 … 日本語以外のWindowsやAppVayor上での出力が文字化けする
  • 変更後: ASCII英語出力 … 動作環境を問わずメッセージ表示が行えるはず

変更内容

  • MakefileMake.cpp の文字コードを変更: SJIS -> UTF-8(BOM有り)
  • 日本語での出力メッセージを全て英語に書き換え
  • ついでにいくつかバグ修正

バグ修正内容

  • __MINGW32__ が定義されている場合に独自定義されている fopen_s の戻り値を void から int に変更(本処理で fopen_s の戻り値を見ているところがあるので、このようにしないと MinGW 環境でビルド通らないはず)
  • printf("Error: makefile[%s]を削除出来ません\n", tmp_file); … 引数間違いがあったので修正

それぞれ小さな変更なのでコミットは分けていない。

関連 PR

これがマージされた前提でコミットを積んである(#31 で MakefileMake が AppVeyor 上で実行されるようになるので、それが適用されている前提があったほうが本PRの動作確認もしやすい)

@kobake kobake force-pushed the makefilemake-english-print branch from 9142f6d to aef583a Compare June 2, 2018 15:34
kobake added a commit that referenced this pull request Jun 2, 2018
ついでにソースコードの文字エンコーディングを UTF-8(BOM有り) に変更。
ついでにいくつかバグ修正(詳細は #44 を参照)。
@kobake kobake force-pushed the makefilemake-english-print branch from aef583a to 64a8e75 Compare June 2, 2018 16:06
kobake added a commit that referenced this pull request Jun 2, 2018
ついでにソースコードの文字エンコーディングを UTF-8(BOM有り) に変更。
ついでにいくつかバグ修正(詳細は #44 を参照)。
@kobake kobake force-pushed the makefilemake-english-print branch from 64a8e75 to c0ae2b5 Compare June 2, 2018 16:25
kobake added a commit that referenced this pull request Jun 2, 2018
ついでにソースコードの文字エンコーディングを UTF-8(BOM有り) に変更。
ついでにいくつかバグ修正(詳細は #44 を参照)。
@kobake kobake force-pushed the makefilemake-english-print branch from c0ae2b5 to fc19e6f Compare June 2, 2018 17:03
kobake added a commit that referenced this pull request Jun 2, 2018
ついでにソースコードの文字エンコーディングを UTF-8(BOM有り) に変更。
ついでにいくつかバグ修正(詳細は #44 を参照)。
@kobake kobake force-pushed the makefilemake-english-print branch from fc19e6f to 07f3ed6 Compare June 2, 2018 17:05
kobake added a commit that referenced this pull request Jun 2, 2018
ついでにソースコードの文字エンコーディングを UTF-8(BOM有り) に変更。
ついでにいくつかバグ修正(詳細は #44 を参照)。
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

vcxproj間の依存関係指定のやり方は2つあります。
・大元のslnで設定するやり方
・各vcxprojでprojectReferenceを指定するやり方

こういうの↓
https://github.com/berryzplus/sakura/blob/0c59d50f14e6ec31fb182dd2e726c7b1a4634e39/sakura/sakura_lang.vcxproj#L89-L94

個人的には後者のやり方が好きなんですが、vs2017のUIでは設定できなくなってるようです。(プロジェクト参照という.NET系の設定方法だからかも知れない)

ビルドの「依存関係」で設定されるものは前者なので、このままでよいと思います。

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

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

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

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

sakura_core/Makefileは消さなくていいんでしたっけ?

@m-tmatma
Copy link
Member

m-tmatma commented Jun 3, 2018

sakura_core/Makefileは消さなくていいんでしたっけ?

HeaderMake の生成物を消すという方針にするなら消したほうがいいと思いますが、
このプルリクエストの対象外だと思います。

@berryzplus
Copy link
Contributor

berryzplus commented Jun 3, 2018

HeaderMake の生成物を消すという方針にするなら消したほうがいいと思いますが、
このプルリクエストの対象外だと思います。

同じ認識です。

で、HeaderMakeの生成物は「消す」と決めた認識です。 86d411f

このプルリクエストはマージするとして、残タスクを確認したかったです。

ついでにソースコードの文字エンコーディングを UTF-8(BOM有り) に変更。
ついでにいくつかバグ修正(詳細は #44 を参照)。
@kobake kobake force-pushed the makefilemake-english-print branch from 07f3ed6 to eb79163 Compare June 3, 2018 10:18
@kobake
Copy link
Member Author

kobake commented Jun 3, 2018

ちょっと分かりにくくてすみません、このPRで有効なコミットは「MakefileMake 出力メッセージを英語に変更」の1件のみで、それ以外に積まれていたコミットは、動作確認用に #31 の内容を取り込んでいただけです。

@kobake
Copy link
Member Author

kobake commented Jun 3, 2018

sakura_core/Makefileは消さなくていいんでしたっけ?

MakefileMake の挙動は「既存の Makefile を読み取って書き換える」処理になっているので、単に Makefile を消してしまうとうまく動作しません。

あまり美しい処理構造ではないのですが、とりあえず本PRの目的は MakefileMake の出力メッセージを英語にすることに絞らせていただき、Makefile のメンテナンス方法自体については別件として処理させてください。

@kobake kobake changed the title [WIP] MakefileMake 出力メッセージを英語に変更 MakefileMake 出力メッセージを英語に変更 Jun 3, 2018
@berryzplus
Copy link
Contributor

kefileMake の挙動は「既存の Makefile を読み取って書き換える」処理になっているので、単に Makefile を消してしまうとうまく動作しません。

了解です。
Makefileは消さない方向で(-^^-;
他もとくに問題ない認識です。

@kobake
Copy link
Member Author

kobake commented Jun 3, 2018

問題ないようであれば、僕以外のどなたか、マージお願いします~

@m-tmatma
Copy link
Member

m-tmatma commented Jun 3, 2018

今見てます。

@m-tmatma
Copy link
Member

m-tmatma commented Jun 3, 2018

一応指摘書きましたが、対応しても対応しなくてもどちらでもいいです。

@kobake
Copy link
Member Author

kobake commented Jun 3, 2018

一応指摘書きましたが

すみません、指摘内容はどれでしょうか。

ちなみにですが Files changed のところにコメント書くときに「Start a review」ボタンで書き始めたときには「Finish your review」みたいなボタン押さないとコメント確定されないです。以前自分がハマったことあるので念のため。

struct stat st;
int ret = stat( top_dir, &st );
if (ret != 0 || !(st.st_mode & _S_IFDIR)) {
printf("Error: �g�b�v�f�B���N�g��[%s]�����‚���܂���\n", top_dir);
// Error: トップディレクトリ[%s]が見つかりません
printf("Error: Failed to stat TopDirectory[%s].\n", top_dir);
Copy link
Member

Choose a reason for hiding this comment

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

Error: Failed to stat TopDirectory

Error: Failed to locate TopDirectory
とかのほうがいいと思います。

stat は "stat" というシステムコールの名前なので、stat を知らないユーザーに
とっては意味がわからないです。

Copy link
Member Author

Choose a reason for hiding this comment

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

最初 read にしようかと迷っていたのですが、MakefileMake のログを直接眺める立場の人は stat 知ってるかな~と思って stat にしちゃってました。

ご指摘内容は頭に入れておきます。一旦今回は stat でやらせてください。

なお cmake の準備により MakefileMake 自体がすぐに不要になりそうな予感。

@m-tmatma
Copy link
Member

m-tmatma commented Jun 3, 2018

ちなみにですが Files changed のところにコメント書くときに「Start a review」ボタンで書き始めたときには「Finish your review」みたいなボタン押さないとコメント確定されないです。以前自分がハマったことあるので念のため。

押しました。

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.

承認しておきます。

@berryzplus
Copy link
Contributor

決着したようなのでマージします。

@berryzplus
Copy link
Contributor

まった!選択肢が3つあってどれにしたらいいか分からん。

  1. create a merge commit
  2. squash and merge
  3. rebase and merge

1でいいような気がするけど、これmasterにマージされますよね?

@kobake
Copy link
Member Author

kobake commented Jun 3, 2018

基本的には 1 で OK です!どれ選んでも master に入ります。

@berryzplus berryzplus merged commit 66094d8 into master Jun 3, 2018
@kobake kobake deleted the makefilemake-english-print branch June 3, 2018 12:23
@m-tmatma m-tmatma added this to the next release milestone Jun 4, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
ついでにソースコードの文字エンコーディングを UTF-8(BOM有り) に変更。
ついでにいくつかバグ修正(詳細は sakura-editor#44 を参照)。
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…nglish-print

MakefileMake 出力メッセージを英語に変更
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants