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

_get_lazyload_commands is is passing the multicommand to itself instead of the Context #70

Closed
n8pease opened this issue Oct 29, 2020 · 4 comments · Fixed by #101
Closed
Labels

Comments

@n8pease
Copy link

n8pease commented Oct 29, 2020

The signature for click.MutliCommand.list_commands takes a Click.Context:

class MultiCommand:
...
    def list_commands(self, ctx):
        """Returns a list of subcommand names in the order they should
        appear.
        """

click.MultiCommand.get_commands is similar: get_command(self, ctx, cmd_name)

The function in sphinx-click to get commands from a multicommand is passing the multicommand to itself (instead of the Context) in list_commands and get_commands:

def _get_lazyload_commands(multicommand):
    commands = {}
    for command in multicommand.list_commands(multicommand):
        commands[command] = multicommand.get_command(multicommand, command)

    return commands

Instead, it should take the context from its caller and pass it to list_commands:

def _get_lazyload_commands(ctx):
    commands = {}
    for command in ctx.command.list_commands(ctx):
        commands[command] = ctx.command.get_command(ctx, command)

    return commands

I'm using click version 7.0, and can see the same code in the latest master (297150c7566a38fb672828a36b681792513d99b0).

My sphinx-click does not have a __version__ attribute, but I can see the same code cited above in the latest master (6b928ee1b83e3a554d040d19e95bd3063e464c60)

amCap1712 added a commit to amCap1712/sphinx-click that referenced this issue May 23, 2021
@regisb
Copy link
Contributor

regisb commented Mar 14, 2022

I am also observing this issue. In my project, when I attempt to generate docs, I get:

make build BUILD_ARGS="-W"
make[1]: Entering directory '/home/regis/projets/overhang/repos/overhang/tutor/docs'
sphinx-build -b html -a -E -n -W "." "_build/html"
Running Sphinx v4.2.0
building [mo]: all of 0 po files
building [html]: all source files
updating environment: [new config] 42 added, 0 changed, 0 removed
reading sources... [ 57%] reference/cli/tutor                                                                                                                                                                      
Exception occurred:
  File "/home/regis/projets/overhang/repos/overhang/tutor/tutor/commands/cli.py", line 49, in get_commands
    actions.do_action_once("root:load", ctx.params["root"])
TypeError: list indices must be integers or slices, not str
The full traceback has been saved in /tmp/sphinx-err-ien2tje3.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make[1]: *** [Makefile:5: build] Error 2
make[1]: Leaving directory '/home/regis/projets/overhang/repos/overhang/tutor/docs'
make: *** [Makefile:8: html] Error 2

It is occurring as I am trying to implement a custom MultiCommand class to load subcommands from plugins, very much as described in the click docs: https://click.palletsprojects.com/en/latest/commands/#custom-multi-commands

You can see my custom implementation here: https://github.com/overhangio/tutor/pull/599/files#diff-135f3cab863e666db1ab4b3c1a36f125f11501065e0cae258d73d706b5dfae56R30

The context argument (which is here a MultiCommand) that is passed to list_command and get_command is not used by the base Group implementation. Thus, the bug does not occur in most cases.

The fix should be relatively straightforward and I can open a PR. Would you be interested? Is a unit test necessary?

@stephenfin
Copy link
Member

I'd definitely be interested. Tests would be appreciated since they prove things are working as expected. Ideally there would be two commits: one to add (failing) unit test(s) and one to fix the bug and the test. Just try your best and let me know if you really can't figure it out.

@stephenfin stephenfin added the bug label Mar 14, 2022
@stephenfin
Copy link
Member

Fixing this would likely unblock #83 also.

regisb added a commit to regisb/sphinx-click that referenced this issue Apr 14, 2022
A previous fix partially resolve an issue with incorrect command type passed to
list_commands, but the same fix should have been applied to get_command as
well.

Close click-contrib#70 (again).
@regisb
Copy link
Contributor

regisb commented Apr 14, 2022

Unfortunately the proposed fix does not 100% resolve this issue. The context must also be passed to get_command, as suggested above. I opened #101 to resolve this.

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 a pull request may close this issue.

3 participants