-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update help message comments. Closes #517 #527
Conversation
@externl can you Include the output of |
|
It looks weird that the last dot is stripped in the output. |
Yeah, not sure what we can do about that. I haven't look to see if maybe that's standard? |
TL;DR all tools do their own things with periods apparently.
|
Should we still remove the |
The version flag actually comes from |
src/command_line.rs
Outdated
/// Define a preprocessor definition. Preprocessor definitions do not have an associated value. | ||
/// This option can be repeated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about: "Comma separated list of preprocessor symbols that will be defined for the compilation"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't support comma separated lists, if you want to define multiple symbols, each one needs a -D
in front of it:
okay: slicec-cs.exe test.slice -D foo -DBar
bad: slicec-cs.exe test.slice -D foo,bar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mm ok, the --help output posted earlier mentioned the comma thing
Define a preprocessor definition. Preprocessor definitions do not have an associated value. Multiple values can be specified by using multiple `-D|--definition` options or by using a comma
src/command_line.rs
Outdated
#[arg(required = true)] | ||
pub sources: Vec<String>, | ||
|
||
/// Files that are needed for referencing, but that no code should be generated for. | ||
/// Reference Slice files or directories containing Slice files. Reference files are used to resolve definitions in | ||
/// the Slice sources being compiled. Directories are searched recursively. | ||
#[arg(short = 'R', long, num_args = 1, action = Append)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good, a sorter version can be "Specify additional directories containing Slice files or Slice files to resolve Slice definitions during compilation"
My preference would be no long version. And if we need a long version, it should be
The message should be consistent: Print or Prints, Define or Defines?
I would call it Can you have multiple |
I'd lean towards keeping both a short and long version, and let users decide what they want.
I think our messages should be written in present-single tense:
Yes: |
I've changed this PR to be a draft. I need to read the clap docs and figure out the correct way to fix the output.
|
If Clap becomes more of a liability than an aid, I think it would be pretty trivial to write a simple command line parser ourselves. We already have all the structure and logic for parsing slice files, comments, and preprocessor directives; command line options can't be any harder than any of those things. Like, Clap is neat, but we're perfectly capable of replacing it with a simple hand-written parser of our own. |
I don't think Clap is the problem, it's just setup wrong. |
Here's the latest output:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, just some nitpicks about the wording of some messages!
Co-authored-by: Austin Henriksen <austin.r.henriksen79@gmail.com>
Co-authored-by: Austin Henriksen <austin.r.henriksen79@gmail.com>
Co-authored-by: Austin Henriksen <austin.r.henriksen79@gmail.com>
This PR updates the generated help messages. Both
-h
and--help
are still the same, clap_derive seems to generate this automatically. I think it's fine to keep for now.Closes #517