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

Allow multiple --proto_path #94

Merged
merged 2 commits into from
Aug 23, 2019
Merged

Conversation

pcj
Copy link
Contributor

@pcj pcj commented Jun 23, 2019

Modifies the CLI arguments to allow multiple --proto_path flags that are propagated to the protocol compiler.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 23, 2019
@pcj pcj force-pushed the proto_paths branch 2 times, most recently from a07a9e3 to c12a7a7 Compare June 23, 2019 19:01
@mingzhi
Copy link
Contributor

mingzhi commented Jun 23, 2019

I am not sure if package cli supports multiple presence of --proto_path (e.g., --proto_path=a --proto_path=b. But protoc supports --proto_path=a:b:., which corresponds to --proto_path=a --proto_path=b --proto_path=.. Thanks!

@pcj
Copy link
Contributor Author

pcj commented Jun 23, 2019

The urfave/cli library supports repeatable flags via the StringSlice flag type.

I've always used multiple -I (--proto_path) flags to protoc rather than folding them into a colon-delimited string, but I guess that should work too.

Name: "proto_path",
Value: ".",
Usage: "the directory in which for protoc to search for imports",
Value: &cli.StringSlice{"."},
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the behavior of this value? If the user specifies value(s) will they override this slice or append to it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since 2016, it overrides: urfave/cli#160

@lukesneeringer lukesneeringer merged commit 561452f into googleapis:master Aug 23, 2019
rambleraptor added a commit to rambleraptor/api-linter that referenced this pull request Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants