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

naga-cli: Support reading input from stdin #1701

Merged
merged 1 commit into from
Jan 27, 2022
Merged

Conversation

rhysd
Copy link
Contributor

@rhysd rhysd commented Jan 26, 2022

Fixes #1700

Three changes are added to support reading from stdin:

  • With argh crate, the INPUT argument cannot be optional. So this PR replaces argh with clap crate.
  • The INPUT argument is now optional: naga [OPTIONS] [FILES...]
  • Since a file path cannot be known when reading from stdin, so a new option --stdin-file-path <PATH> is added to notify the file path. This is the same way as eslint does.

@kvark
Copy link
Member

kvark commented Jan 26, 2022

With argh crate, the INPUT argument cannot be optional.

I don't think this reasoning is really sound. All it would take is to change this:

    /// the input file
    #[argh(positional)]
    input: String,

    /// the output file. If not specified, only validation will be performed
    #[argh(positional)]
    output: Vec<String>,

Into this:

    /// the input and output files
    #[argh(positional)]
    files: Vec<String>,

Would you be open to refactor the PR on top of argh?

@rhysd
Copy link
Contributor Author

rhysd commented Jan 27, 2022

Would you be open to refactor the PR on top of argh?

That also makes sense to me. I will update this patch today.

@rhysd
Copy link
Contributor Author

rhysd commented Jan 27, 2022

@kvark I updated this patch to keep argh dependency and force pushed the branch. Could you review the changes?

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Nice!

@kvark kvark merged commit 6842db4 into gfx-rs:master Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

naga-cli: Support reading input from stdin
2 participants