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

Improve slice compiler help #517

Closed
pepone opened this issue Apr 12, 2023 · 9 comments · Fixed by #527
Closed

Improve slice compiler help #517

pepone opened this issue Apr 12, 2023 · 9 comments · Fixed by #527
Assignees
Labels
enhancement New feature or request question Further information is requested
Milestone

Comments

@pepone
Copy link
Member

pepone commented Apr 12, 2023

slicec-cs -h
The slicec-cs compiler, for compiling Slice files into C# code.


Usage: slicec-cs [OPTIONS] <SOURCES>...

Arguments:
  <SOURCES>...  List of slice files to compile

Options:
  -R, --references <REFERENCES>
          Files that are needed for referencing, but that no code should be generated for
  -D, --definitions <DEFINITIONS>
          Preprocessor Symbols defined on the command line
  -w, --warn-as-error
          Instructs the compiler to treat warnings as errors
      --allow-warnings <ALLOW_WARNINGS>
          Instructs the compiler to allow warnings. Specify a list of warnings to allow, or leave empty to allow all warnings
      --dry-run
          Validates input files without generating code for them
      --output-dir <OUTPUT_DIR>
          Output directory for generated code, defaults to the current working directory
      --diagnostic-format <DIAGNOSTIC_FORMAT>
          Output format for emitted errors [default: human] [possible values: human, json]
      --disable-color
          Disables ANSI escape code for diagnostic output
  -h, --help
          Print help (see more with '--help')
  -V, --version
          Print version
  • The -R,--references option description is a bit vague, it would be hard to guess that you can use a directory here.
  • Does the -D option accepts a list of definitions, how are the symbols separated? Or do you have to specify -DFOO -DBAR etc, the description doesn't make it clear.
  • What does it mean to allow a warning?
  • The distinction between -h and --help seems uncommon, do we really need both? There are not that many things to document here, I think one version will be enough.
@pepone pepone added question Further information is requested enhancement New feature or request labels Apr 12, 2023
@pepone pepone added this to the 0.1 milestone Apr 12, 2023
@ReeceHumphreys
Copy link
Contributor

Moving this to icerpc/slicec. The messages are generated from the doc comments on the SliceOptions fields in command_line.rs

@ReeceHumphreys ReeceHumphreys transferred this issue from icerpc/icerpc-csharp Apr 12, 2023
@externl
Copy link
Member

externl commented Apr 12, 2023

We should also remove -V, --version should be sufficient.

@ReeceHumphreys
Copy link
Contributor

I think having -h and --help is fine. Seems to be the default behavior from clap

@pepone
Copy link
Member Author

pepone commented Apr 12, 2023

I think having -h and --help is fine. Seems to be the default behavior from clap

I don't think it is practical to have this difference when the output is almost the same, it would make sense if the --help version is much more detailed than the output for -h.

@ReeceHumphreys
Copy link
Contributor

ReeceHumphreys commented Apr 12, 2023

I think having -h and --help is fine. Seems to be the default behavior from clap

I don't think it is practical to have this difference when the output is almost the same, it would make sense if the --help version is much more detailed than the output for -h.

Seems that there is a field we could be using for long help. Im guessing since we do not specify anything currently it just uses the same for both.
https://docs.rs/clap/latest/clap/_derive/index.html#arg-attributes

@bernardnormier
Copy link
Member

Should -w be -W? It would be consistent with -R and -D.

Not that -w is clang means:

Disable all diagnostics.

!

@bernardnormier
Copy link
Member

       --allow-warnings <ALLOW_WARNINGS>
          Instructs the compiler to allow warnings. Specify a list of warnings to allow, or leave empty to allow all warnings
       ...
      --diagnostic-format <DIAGNOSTIC_FORMAT>
          Output format for emitted errors [default: human] [possible values: human, json]

Could we give possible values for --allow-warnings too?

@ReeceHumphreys
Copy link
Contributor

Not that -w is clang means:

Seems that rustc uses:

-W <foo>          Warn about <foo>
-A <foo>           Allow <foo>
-D <foo>           Deny <foo>

To treat all warnings as errors you would then use

rustc --deny warnings

@ReeceHumphreys
Copy link
Contributor

Could we give possible values for --allow-warnings too?

We get the possible values for DiagnosticFormat since it implements [derive(ValueEnum)]. If we create an enum with the warning names once we switch over to using those we would able to then derive ValueEnum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants