-
Notifications
You must be signed in to change notification settings - Fork 184
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
node graph generate
accepts multiple ROOT_NODES
#6338
Conversation
This is a known problem of command line interfaces that have an option that accepts multiple values. The parser will be greedy and treats other (optional) positional arguments as a value for the option. The distinction has to be made explicitly by the caller. You can insert the |
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.
Thanks @bilke . As you noticed changing the positional arguments to take multiple values doesn't play well with the optional output filename also being positional. See my comments elsewhere for a potential solution.
ce1979a
to
64e713e
Compare
TODO: output path must explicitly stated.
root nodes. As suggested by @sphuber.
This is necessary because otherwise if a user wants to specify the output file when using multiple root nodes, they would have to specify a single root node again or the output file would be mistaked for the deprecated root node argument: verdi node graph generate -N 1 2 -- 1 output.pdf
64e713e
to
0fb1ff9
Compare
Thanks a lot for the contribution @bilke |
…eam#6338) So far, the command only allowed to specify a single root node through a positional argument. However, in some cases having multiple root nodes are perfectly sensible. Unfortunately, simply changing the positional argument to accept multiple nodes won't work because the command also accepts a second optional positional argument to specify the output filename. Making the root node accept multiple values will make it greedy and it will always mistake the output filename as another root node specifier. As an alternative solution the `-N/--nodes` and `-O/--output-file` options are added that replace the positional arguments that are now deprecated. Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
…eam#6338) So far, the command only allowed to specify a single root node through a positional argument. However, in some cases having multiple root nodes are perfectly sensible. Unfortunately, simply changing the positional argument to accept multiple nodes won't work because the command also accepts a second optional positional argument to specify the output filename. Making the root node accept multiple values will make it greedy and it will always mistake the output filename as another root node specifier. As an alternative solution the `-N/--nodes` and `-O/--output-file` options are added that replace the positional arguments that are now deprecated. Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Implements #6335.
Related to https://aiida.discourse.group/t/node-graph-generate-with-multiple-root-nodes/353.
TODO
Current usage is
if the output is left out then the last root node name would be used as the output file name.
@sphuber Do you have an idea / opinion how the cli interface should handle that?