Skip to content

Add --directory option #2876

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

Merged
merged 8 commits into from
Feb 12, 2021
Merged

Add --directory option #2876

merged 8 commits into from
Feb 12, 2021

Conversation

sigurdm
Copy link
Contributor

@sigurdm sigurdm commented Feb 8, 2021

Fixes: #2864

@google-cla google-cla bot added the cla: yes label Feb 8, 2021
@sigurdm sigurdm requested a review from jonasfj February 9, 2021 13:33
@@ -74,6 +74,8 @@ class AddCommand extends PubCommand {

argParser.addFlag('precompile',
help: 'Precompile executables in immediate dependencies.');
argParser.addOption('directory',
abbr: 'C', help: 'Run this in <dir>.', valueHelp: 'dir');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
abbr: 'C', help: 'Run this in <dir>.', valueHelp: 'dir');
abbr: 'C', help: 'Run in the directory <dir>.', valueHelp: 'dir');

argParser.addOption(
'directory',
abbr: 'C',
help: 'Run the subcommand in <dir>.',
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I suggest Run in the directory <dir>.

@@ -109,6 +112,13 @@ class PubCommandRunner extends CommandRunner<int> implements PubTopLevel {
});
argParser.addFlag('verbose',
abbr: 'v', negatable: false, help: 'Shortcut for "--verbosity=all".');
argParser.addOption(
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, does it have to both be a top-level argument, and an argument in all the subcommands?

What happens if it's passed twice...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm - it doesn't have to. That just made more sense to me. I would personally instinctively give it at the top-level (it is an option that changes how the following command is interpreted), but it would be annoying to have not work for each command.

We could do it either way if you think it is better.

@sigurdm sigurdm merged commit 178f2ed into dart-lang:master Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add -C option to run pub in another directory
2 participants