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

Help text duplicated when using more than 1 CLI instance #1002

Open
kmvanbrunt opened this issue Sep 28, 2020 · 10 comments
Open

Help text duplicated when using more than 1 CLI instance #1002

kmvanbrunt opened this issue Sep 28, 2020 · 10 comments
Assignees

Comments

@kmvanbrunt
Copy link
Member

import sys
from cmd2 import cmd2

instance_1 = cmd2.Cmd()
instance_2 = cmd2.Cmd()
sys.exit(instance_2.cmdloop())

With this program, run alias -h. The subcommand help gets repeated.

  SUBCOMMAND
    create    create or overwrite an alias
    delete    delete aliases
    list      delete aliases
    create    create or overwrite an alias
    delete    delete aliases
    list      delete aliases

Since the parsers (e.g. Cmd.alias_parser) are class objects, they get updated with each instantiation. This problem has existed since we added the argparse decorators, but it's being exercised a bit more since we added CommandSet and as_subcommand_to decorators. This specific issue of repeated text was introduced when cmd2.py started using the as_subcommand_to decorator.

For the 2.0 release, we need to find a way to make deep copies of the parsers and tie each copy to an instance. We won't have 1 parser attached to the functions anymore. Instead the decorators will need to retrieve the instance-specific parser.

@kmvanbrunt kmvanbrunt added this to the 2.0.0 milestone Sep 28, 2020
@tleonhardt
Copy link
Member

I have a very vague recollection of researching how to make a deep copy of a parser and finding that it was possible in Python 3.7+ (or something similar) but not beforehand.

@tleonhardt
Copy link
Member

Yeah, inability to deep-copy an ArgumentParser was fixed in Python 3.7. The context is in older issue #522

@tleonhardt
Copy link
Member

@kmvanbrunt #1005 appears to have fixed this, at least temporarily. Are we awaiting a more permanent fix? Or should this issue be closed?

@kmvanbrunt
Copy link
Member Author

@tleonhardt The permanent fix needs to be in 2.0. We need a way to support instance-specific parsers and that will most likely be a breaking change.

@kmvanbrunt
Copy link
Member Author

@anselor Do you have any thoughts on this issue?

@anselor
Copy link
Contributor

anselor commented Nov 20, 2020

I'm sure I do I've just been busy with other more immediate deadlines and haven't had time to come back to this and other cmd2 stuff.

@kmvanbrunt
Copy link
Member Author

@anselor and I agree the 2.0 release does not have to wait for this fix. The risk of someone using two cmd2 objects and editing common parsers between them is pretty low.

@kmvanbrunt kmvanbrunt removed this from the 2.0.0 milestone Jan 21, 2021
@tleonhardt
Copy link
Member

@kmvanbrunt Is this fixed now? Or are we waiting on some better fix?

@federicoemartinez
Copy link
Contributor

Could this be avoided if the decorators accepted a argparser factory (a callable that returns an instance of argparser) so the instance is created when needed?

@anselor
Copy link
Contributor

anselor commented Sep 25, 2023

I tried this today to be sure but I was pretty sure this suggestion by itself wouldn't work because the decorator, just like the the ArgumentParser instances, only runs once during the class declaration.

I've been digging around the code a bit and I think may have a solution uses the factory suggestion while also maintaining backwards compatibility. I think what we need to do is, as a command is registered in the commandset (be it during construction or later) we need to at that point either clone the provided ArgumentParser or call the factory method and then store that in the Cmd or CommandSet instance. The innermost cmd_wrapper() method inside of with_argparser() would instead look up the ArgumentParser instance from the provider_self rather than the one passed in the decorator.

@federicoemartinez

anselor added a commit that referenced this issue Sep 26, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 26, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 26, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 26, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 26, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 26, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 26, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
@anselor anselor self-assigned this Sep 26, 2023
anselor added a commit that referenced this issue Sep 27, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 27, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Sep 27, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
anselor added a commit that referenced this issue Oct 30, 2023
…rgumentParser or a

factory callable that returns an ArgumentParser.
Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable
or by deep-copying the provided ArgumentParser.
With this change a new argparse instance should be created for each instance of Cmd.

Addresses #1002
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants