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

Change -x and -y options to have their argument optional #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

UffeJakobsen
Copy link

Options -x and -y becomes reporting options - When no NS argument is supplied

Options -x and -y becomes reporting options -  When no NS argument is supplied
@UffeJakobsen
Copy link
Author

Did you have a chance to look at this pull ? :-)

@dlenski
Copy link
Owner

dlenski commented Sep 23, 2020

Thanks for the reminder. I like the idea, but the problem is that nargs='?' is extremely unintuitive in its actual behavior.

#!/usr/bin/env python3

import argparse
p = argparse.ArgumentParser()
p.add_argument('filenames', nargs='*')
p.add_argument('--test', nargs='?')
p.add_argument('--other', action='store_true')

args1 = p.parse_args(['foo','bar','--test'])
print(args1)

args2 = p.parse_args(['--test=testARG','foobar'])
print(args2)

args3 = p.parse_args(['--test','testARG','foobar'])
print(args3)

args4 = p.parse_args(['--test','--other'])
print(args4)

args5 = p.parse_args(['--test','will_this_be_the_argument_of_test_or_a_filename?','--other'])
print(args5)

Output:

Namespace(filenames=['foo', 'bar'], other=False, test=None)
Namespace(filenames=['foobar'], other=False, test='testARG')
Namespace(filenames=['foobar'], other=False, test='testARG')
Namespace(filenames=[], other=True, test=None)
Namespace(filenames=[], other=True, test='will_this_be_the_argument_of_test_or_a_filename?')

In other words, an option with nargs='?' does not provide a reasonably mechanism to signal that whatever follows that option on the command-line argument is not intended as the argument of that option.

Because of this extremely unintuitive behavior, and because there is no better way to create an "argument-optional" option using the Python standard library… I don't want to use nargs='?' within wtf. 🤷‍♂️

@dlenski
Copy link
Owner

dlenski commented Sep 23, 2020

If you can think of a simple way to implement "argument-optional" options that doesn't exhibit the surprising behavior of the args5 case above, and doesn't use anything outside the standard library… I'm definitely open to it. :-)

@UffeJakobsen
Copy link
Author

@dlenski: Thanks for your reply :-)

I understand your concern about nargs='?'...

Changing the input files from nargs='*' into nargs=argparse.REMAINDER will solve parts of the problem by forcing the files into always appearing as the last elements on the cmd line - but it will just open new problems....
The nargs=argparse.REMAINDER is handled somewhat strange inside argparse (default does not work for it - and the '--' separator between options and arguments is not handled by argparse anymore)

Another more logical approach - would be introducing -X and -Y as options that only does reporting and no changes compared to their lower-case equivalents.
That approach is in line with your current semantics (upper-case options are reporting options - lower-case options changes output)

Only problem with that is that the -X option is already used to exit without any errorcode

Question is if you would accept changing the existing -X option to another one ?

@dlenski
Copy link
Owner

dlenski commented Oct 1, 2020

Another more logical approach - would be introducing -X and -Y as options that only does reporting and no changes compared to their lower-case equivalents.
That approach is in line with your current semantics (upper-case options are reporting options - lower-case options changes output)

Only problem with that is that the -X option is already used to exit without any errorcode

Question is if you would accept changing the existing -X option to another one ?

Yeah, this seems like the most reasonable approach. Good ideas. How about change the existing -X to… hrm, -N?

@UffeJakobsen
Copy link
Author

Great - I'll rework the pull request :-)

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

Successfully merging this pull request may close these issues.

2 participants