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

CLI: do not except when --help is passed to sub command #116

Merged
merged 2 commits into from
Mar 25, 2022

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Mar 24, 2022

Fixes #97

The rocrate add workflow --help call would except because the add
command group would attempt to instantiate a research object crate. But
since no existing crate was defined through the -c option, the
instantiation of the crate would except.

There is no straightforward way in click to prevent the execution of
group command code when --help is passed. Instead, we move the
addition of the -c/--crate-dir option to those commands that actually
need it. Code duplication is prevented by defining the option once,
assigning it to OPTION_CRATE_PATH and using that decorator for each
command that requires the option.

This is a breaking change for the interface as the crate path should now
no longer be specified directly after the main command but rather after
the actual leaf command. So:

rocrate -c some/ro-crate add workflow

Now becomes

rocrate add workflow -c some/ro-crate

The advantage of this is that it doesn't enforce this option on all
commands, even those that don't need it and it is probably more
intuitive for the user to put the option closer to all other options.

@sphuber
Copy link
Contributor Author

sphuber commented Mar 24, 2022

I ran into this problem and saw there was an open issue. I tried to find a solution first with keeping the place of the option as it was, but I think this is not possible with the current functionality of click.

The `rocrate add workflow --help` call would except because the `add`
command group would attempt to instantiate a research object crate. But
since no existing crate was defined through the `-c` option, the
instantiation of the crate would except.

There is no straightforward way in `click` to prevent the execution of
group command code when `--help` is passed. Instead, we move the
addition of the `-c/--crate-dir` option to those commands that actually
need it. Code duplication is prevented by defining the option once,
assigning it to `OPTION_CRATE_PATH` and using that decorator for each
command that requires the option.

This is a breaking change for the interface as the crate path should now
no longer be specified directly after the main command but rather after
the actual leaf command. So:

    rocrate -c some/ro-crate add workflow

Now becomes

    rocrate add workflow -c some/ro-crate

The advantage of this is that it doesn't enforce this option on all
commands, even those that don't need it and it is probably more
intuitive for the user to put the option closer to all other options.
@simleo
Copy link
Collaborator

simleo commented Mar 25, 2022

Awesome, thanks! I've re-wrapped the docstrings to stay within 80 columns for readability

@simleo simleo merged commit 63e8d41 into ResearchObject:master Mar 25, 2022
@simleo
Copy link
Collaborator

simleo commented Mar 25, 2022

@sphuber I forgot: please add the relevant copyright info to the files you modified, in the boilerplate at the top

@sphuber
Copy link
Contributor Author

sphuber commented Mar 25, 2022

I forgot: please add the relevant copyright info to the files you modified, in the boilerplate at the top

I am not sure I understand what you mean. What should I change or add?

@sphuber sphuber deleted the fix/97/cli-help branch March 25, 2022 16:13
@simleo
Copy link
Collaborator

simleo commented Mar 25, 2022

At the top of code files there's a copyright notice that looks like this:

# Copyright 2019-2022 The University of Manchester, UK
[...]
# Licensed under the Apache License, Version 2.0 (the "License");
[...]

Please add a line like the following:

# Copyright 2022 <YOUR EMPLOYER>

@sphuber
Copy link
Contributor Author

sphuber commented Mar 25, 2022

See #117

@simleo
Copy link
Collaborator

simleo commented Mar 28, 2022

I just recalled that we're adding all copyright entries to all files, for simplicity. I've updated #117 accordingly.

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.

CLI can generate an error when command help is requested
2 participants