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

fix: fixes org not being parsed for subcommands #724

Closed

Conversation

someguynamedmatt
Copy link

Relates to Issue #723

Following the docs for creating a new release was giving errors despite passing in the requested org command-line arg. I (think I) tracked it down to new not expecting the organization in its argument parsing despite searching for it on invocation.

Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

Thank @someguynamedmatt, I'm afraid this fix duplicates the org argument, however. clap has a global flag on arguments, which would usually be used for this. I believe that in releases, we can set the org argument to global(true).

Edit: I hadn't looked at the code, of course the flag would go here:

clap::Arg::with_name("org")

Some background: The reason we did not do this initially is that we wanted to keep it consistent between --org and --project. For --project, this does not work, since the -p shorthand is reused between commands where sometimes multiple projects are allowed, and sometimes only one.

@someguynamedmatt
Copy link
Author

@jan-auer Updated. Thanks for looking at this. I've changed the code slightly to check the top-level command first then recursively checking the subcommands for the org value.

@jan-auer
Copy link
Member

I've changed the code slightly to check the top-level command first then recursively checking the subcommands for the org value.

Thanks! This surprises me, but it seems that we're hitting clap-rs/clap#1570. With global(true) the value should just magically appear without changes in get_org. However, due to the bug, you'd have to make those changes.

Would you mind waiting until the upstream issue gets resolved? Otherwise, we'd have to check at every nesting level of subcommands, which seems rather dirty.

@someguynamedmatt
Copy link
Author

@jan-auer Oh, yeah, that would be a much better solution. I was wondering why global didn't seem to solve the problem. At least it's a known issue. If that upstream fix fixes this issue, all the better.

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