Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

group return value is ignored #2124

Closed
arthaud opened this issue Nov 2, 2021 · 5 comments · Fixed by #2217
Closed

group return value is ignored #2124

arthaud opened this issue Nov 2, 2021 · 5 comments · Fixed by #2217
Assignees
Milestone

Comments

@arthaud
Copy link

arthaud commented Nov 2, 2021

Calling a command defined as @click.group() with standalone_mode=False does not propagate the return value as expected.

For instance:

import click
import sys

@click.group(invoke_without_command=True)
@click.option("-l", "--local-configuration", type=str)
def main(local_configuration):
    print('Inside main')
    return 1

if __name__ == '__main__':
    print(main(sys.argv[1:], standalone_mode=False))
$ python test.py
Inside main
None

It does work when the command is defined with @click.command(), hence I suppose this is a bug.

Environment:

  • Python version: 3.7.5
  • Click version: 8.0.3
@jcrotts
Copy link
Contributor

jcrotts commented Nov 6, 2021

Thanks for opening the issue, I ran a git bisect to see where this behavior changed.
It showed 079f91a as when the behavior changed I'm not sure why moving to a src directory would cause this.

@rohori
Copy link

rohori commented Nov 8, 2021

It seems related to

click/src/click/core.py

Lines 1633 to 1638 in d5932d6

# No subcommand was invoked, so the result callback is
# invoked with None for regular groups, or an empty list
# for chained groups.
with ctx:
super().invoke(ctx)
return _process_result([] if self.chain else None)

where return value of super().invoke(ctx) is replaced with [] or None before passed to _process_result. Return value propagation (without subcommand invocation) stops here with invoke_without_command=True, regardless of standalone_mode.

@davidism
Copy link
Member

davidism commented Mar 2, 2022

Not sure what to do with this one. The difference in behavior between groups and commands wasn't documented, and it's not clear why group returns weren't used when adding support for command return values. I think it could make sense to use the group's return value as well though. The problem is making sure existing code continues to work after the change, at least for a deprecation period.

The return value of a command is the return value, if any. The return value of a group is the result of calling its result_callback on the return value(s) of it subcommand(s). result_callback only gets the subcommand result, it won't get a value returned by the group itself.

  • A group can be invoked without a subcommand, and returns a value. Would the value be returned directly, or still passed through the result callback? What if the callback only expects values returned by subcommands?
  • A subcommand is required, and only the subcommand returns a value. This is the current expected behavior.
  • A subcommand is required, and both the group and the subcommand return values. This is tricky because the result callback would only be expecting one value based on current behavior. Should the subcommand value overwrite the group value?
  • The group is in chain mode, the return value of each subcommand is collected in a list. Should the return value of the group be the first item in the list? What if the group doesn't return anything, and the result callback expects that every item is not None (that's how the example in the docs works)?

A possible solution that sidesteps the questions above: only use the group return value if invoked without subcommand. That is, if the group is treated like a command itself, return its value as if it were a command.

Regardless of the solution, the behavior of return values needs to be much better documented.

For now, you could add a result callback to the group that returns what you would have returned from the group if it was used without a subcommand.

@main.result_callback
@click.pass_context
def group_result(ctx, value):
    if ctx.invoked_subcommand is None:
        return 1

@davidism davidism changed the title click.group with standalone_mode=False does not propagate the return value group return value is ignored Mar 2, 2022
@davidism
Copy link
Member

davidism commented Mar 2, 2022

For #1178, in 8.0 we changed the behavior so that the result callback was called even if there was no subcommand, passing None.

Previously, the group return value was returned, as long as the group wasn't in chain mode and was called without a subcommand. In chain mode, the group return was always ignored since the intention in that mode was to have a pipeline of return values.

The behavior before 8.0 was basically what I just described as a possible solution to this issue. Now that I've looked over this more, I think that makes more sense, I think the change should be reverted in 8.1.

@davidism
Copy link
Member

davidism commented Mar 2, 2022

A compromise that should satisfy that issue and this one would be to pass the group return value to the result callback if it's invoked directly. As shown in the workaround I posted above, you can tell whether the value is from a subcommand or not based on ctx.invoked_subcommand.

@davidism davidism added this to the 8.1.0 milestone Mar 2, 2022
@davidism davidism self-assigned this Mar 19, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants