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

入力ソースコードファイルの文字コードを指定できるように #8

Merged
merged 1 commit into from
Jan 16, 2022

Conversation

meltingrabbit
Copy link
Collaborator

概要

入力ソースコードファイルの文字コードを指定できるように

Issue

詳細

C2AのUTF-8化に伴い追加

検証結果

Copy link

@yngyu yngyu left a comment

Choose a reason for hiding this comment

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

これ pytest 通らなくなる気がするんですが、c2a-core の pytest の修正 or テストって行いましたか?
加えて core と user で encoding が違うと死ぬ...?

@yngyu
Copy link

yngyu commented Jan 16, 2022

あーいや Enumは流石に全部ascii文字しかないからutf-8だろうがshift-jisだろうが変わらないんでしょうか

@yngyu
Copy link

yngyu commented Jan 16, 2022

ただ引数が増えてるのでそっちの修正は必要な気がします、お願いします

@sksat
Copy link
Collaborator

sksat commented Jan 16, 2022

引数増えてる,はちゃんと増えてそう?
https://github.com/ut-issl/c2a-core/pull/185/files#diff-7aec042ede215a62922325c00ba215318b063a32d9b2e2120f6530333816fee9

@sksat
Copy link
Collaborator

sksat commented Jan 16, 2022

enumの部分がASCIIだけなら実際にはいける気はするけれど,それはそれとしてcoreとuser両方のencoding指定すべきな気はするなあ

@yngyu
Copy link

yngyu commented Jan 16, 2022

あーすいません、ありがとうございます...

@meltingrabbit
Copy link
Collaborator Author

pytest

通したよ~

ISSL MOBCの方は,

https://github.com/ut-issl/c2a-core/blob/4379331b8804da3a0ad41d5c30c3fb64a85eb757/Examples/minimum_user_for_s2e/src/src_user/Test/utils/c2a_enum_utils.py#L22

をsjisに設定することになります.

@yngyu (と言うわけで,よさそうならapproveいただけると. あと ut-issl/c2a-core#184 も)

@sksat
Copy link
Collaborator

sksat commented Jan 16, 2022

あー,userがsjisの場合ではどうせビルド時にはcoreのutf8->sjis変換してるからこのスクリプトが読みに行く時はsjis仮定でいい(しそうすべきですらある)のか.じゃあよさそう.

@meltingrabbit
Copy link
Collaborator Author

あー,userがsjisの場合ではどうせビルド時にはcoreのutf8->sjis変換してるからこのスクリプトが読みに行く時はsjis仮定でいい(しそうすべきですらある)のか

そうそう.userとcoreは流石に一貫してほしい.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants