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

Add the "codegen" subcommand #98

Merged
merged 6 commits into from
Feb 16, 2019
Merged

Conversation

kmyk
Copy link
Contributor

@kmyk kmyk commented Feb 15, 2019

ユーザインターフェースは以下のようになりました。
gen サブコマンドのものを拝借してできるだけ似せた形です。
言語の指定は、デフォルトではconfigファイルを読みに行きます。

$ ./atcoder-tools codegen -h
usage: ./atcoder-tools codegen [-h] [--without-login] [--lang LANG]
                               [--template TEMPLATE] [--save-no-session-cache]
                               [--config CONFIG]
                               url

positional arguments:
  url                   URL (e.g. https://atcoder.jp/contests/abc012/tasks/abc012_3)

optional arguments:
  -h, --help            show this help message and exit
  --without-login       Download data without login
  --lang LANG           Programming language of your template code, cpp or java or rust.
                        [Default] cpp
  --template TEMPLATE   File path to your template code
                        [Default (C++)] /home/user/GitHub/atcoder-tools/atcodertools/tools/templates/default_template.cpp
                        [Default (Java)] /home/user/GitHub/atcoder-tools/atcodertools/tools/templates/default_template.java
                        [Default (Rust)] /home/user/GitHub/atcoder-tools/atcodertools/tools/templates/default_template.rs
  --save-no-session-cache
                        Save no session cache to avoid security risk
  --config CONFIG       File path to your config file
                        [Default (Primary)] /home/user/.atcodertools.toml
                        [Default (Secondary)] /home/user/GitHub/atcoder-tools/atcodertools/tools/atcodertools-default.toml
  • 実装の重複を防ぐために gen サブコマンドの実装の内部の関数を借りてきてます。コピペを嫌って直接 import してますが、これはこれで依存が後々つらいので後で refactoring が必要です。
  • コード自動生成機能だけ利用できるコマンドがほしい #96 (comment) で「code generator をオプションで指定したい」という話が出ていますが、これは今のところしていません。書き終わってから気付いたことと、足すとすれば gen コマンドに足すのと同時に行なうべきだろうことが理由です。必要なら別のプルリクとして投げます。
  • template のオプションでの指定はできます。
  • テストがひとつしかなくてさみしいですが、下位の機能のコード自動生成部分の wrapper でしかないので、まったく不足しているということもないはずです。

@codecov-io
Copy link

codecov-io commented Feb 15, 2019

Codecov Report

Merging #98 into master will decrease coverage by 0.68%.
The diff coverage is 77.08%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #98      +/-   ##
========================================
- Coverage   91.69%    91%   -0.69%     
========================================
  Files          46     47       +1     
  Lines        1950   2046      +96     
========================================
+ Hits         1788   1862      +74     
- Misses        162    184      +22
Impacted Files Coverage Δ
atcodertools/tools/codegen.py 77.08% <77.08%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa57a15...279b118. Read the comment docs.

@kmyk
Copy link
Contributor Author

kmyk commented Feb 15, 2019

returncode が常に 1 になっちゃってたので修正しました。

@kyuridenamida
Copy link
Owner

kyuridenamida commented Feb 15, 2019

  • 実装の重複を防ぐために gen サブコマンドの実装の内部の関数を借りてきてます。コピペを嫌って直接 import してますが、これはこれで依存が後々つらいので後で refactoring が必要です。

コードは少ないほうがいいし、どうせ手遅れになる前にリファクタするので今はこれでOKです。

  • #96 (comment) で「code generator をオプションで指定したい」という話が出ていますが、これは今のところしていません。書き終わってから気付いたことと、足すとすれば gen コマンドに足すのと同時に行なうべきだろうことが理由です。必要なら別のプルリクとして投げます。

両方同時にサポートされるというのは一理あります。なので、このままで行きましょう。別のissueを立てました (#100)。
この機能へのcode_generatorサポートを要求した理由は次の通りです: 頭の中に思い描いているこのコマンドの利用用途が、何かしらのツールのツールチェインの一部分として利用するというもので、その場合設定ファイルは使われなさそうなので、先立って対応してもらってもいいかなと考えたからです。

  • template のオプションでの指定はできます。

ありがとうございます!

  • テストがひとつしかなくてさみしいですが、下位の機能のコード自動生成部分の wrapper でしかないので、まったく不足しているということもないはずです。

確認させてください。

Copy link
Owner

@kyuridenamida kyuridenamida left a comment

Choose a reason for hiding this comment

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

旧URLのテスト追加とテストのclass名が少し変なのを直して頂ければ他は良さそうです!

tests/test_codegen_command.py Show resolved Hide resolved
tests/test_codegen_command.py Outdated Show resolved Hide resolved
tests/test_codegen_command.py Outdated Show resolved Hide resolved
@kyuridenamida kyuridenamida added this to the 1.1.2 milestone Feb 15, 2019
atcoder-tools Outdated Show resolved Hide resolved
@kmyk
Copy link
Contributor Author

kmyk commented Feb 16, 2019

@kyuridenamida 指摘してもらったところは修正できたはずです。もう一度確認お願いします

@kmyk
Copy link
Contributor Author

kmyk commented Feb 16, 2019

readme に説明を書くの忘れていました。書きます

@kmyk
Copy link
Contributor Author

kmyk commented Feb 16, 2019

書きました

@kyuridenamida kyuridenamida merged commit bb03ef6 into kyuridenamida:master Feb 16, 2019
@kyuridenamida
Copy link
Owner

kyuridenamida commented Feb 16, 2019

マージしました。ありがとうございます

@kmyk
Copy link
Contributor Author

kmyk commented Feb 16, 2019

こちらこそありがとうございます

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