Allow combining short CLI arguments #32
Conversation
|
Thanks for the PR. Looks mostly good and the change is certainly welcome, but I will add some comments. |
This comment has been minimized.
This comment has been minimized.
|
Please commit this import pytest
import sys
def test_argparser(capsys):
from covert.__main__ import argparse # Import *after* capsys wraps stdout/stderr
# Correct but complex arguments
sys.argv = "covert enc --recipient asdf1 -r asdf2 -Arrp recipient1 recipient2".split()
a = argparse()
assert a.recipients == ['asdf1', 'asdf2', 'recipient1', 'recipient2']
assert a.paste is True
assert a.askpass == 1
# Should produce no output
cap = capsys.readouterr()
assert not cap.out
assert not cap.err
# Missing argument parameter
sys.argv = "covert enc -Arrp recipient1".split()
with pytest.raises(SystemExit):
argparse()
cap = capsys.readouterr()
assert not cap.out
assert "Argument parameter missing: covert enc -Arrp …" in cap.errThe tests are passing and the implementation seems to be working. Very close to being complete then, just minor cleanup needed. |
covert-encryption
changed the title
__main__.py
Outdated
| if isinstance(a, str) is True: | ||
| a = [a] | ||
| for i, av in enumerate(a): | ||
| argvar = next((k for k, v in ad.items() if av in v), None) | ||
| if isinstance(av, int) is True: | ||
| continue | ||
| if av is None: | ||
| stderr.write(f'{modehelp}\n 💣 Unknown argument: covert {args.mode} {aprint}\n') | ||
| sys.exit(1) |
There was a problem hiding this comment.
Changed == to is or is not.
There was a problem hiding this comment.
Do not use is True unless when you have to (e.g. to tell 1 and True apart).
__main__.py
Outdated
| if any(longarg[1:] in a for longarg in longargs) is True: | ||
| stderr.write(f' 💣 Multiple arguments cannot have long arguments: covert {args.mode} {a}\n') | ||
| sys.exit(1) | ||
| elif any(arg not in shortargs for arg in list(a[1:])) is True: |
There was a problem hiding this comment.
Remove the is True, it is not needed.
__main__.py
Outdated
| else: | ||
| setattr(args, argvar, True) | ||
| print(var) | ||
| if isinstance(var, int) is not True and var is not None: |
There was a problem hiding this comment.
if not isinstance(var, int) and var is not None:There was a problem hiding this comment.
Looks like what you really wanted here is isinstance(var, list) or isinstance(var, (str, list)) but as commented below, this entire check should be removed.
__main__.py
Outdated
| if any(longarg[1:] in a for longarg in longargs) is True: | ||
| stderr.write(f' 💣 Multiple arguments cannot have long arguments: covert {args.mode} {a}\n') | ||
| sys.exit(1) |
There was a problem hiding this comment.
Should not test for this. -out should be allowed and it means -o -u -t, not --out.
__main__.py
Outdated
| if var[-1].startswith('-') or var[-1].startswith('--'): | ||
| if var[-1][1:] in longargs or var[-1][1:] in shortargs: | ||
| stderr.write(f'{modehelp}\n 💣 Argument parameter cannot be the same with arguments: covert {args.mode} {aprint} {var[-1]} …\n') | ||
| sys.exit(1) |
There was a problem hiding this comment.
Argument parameter CAN be the same as any argument. Remove this check.
Instead of
-A -r recipient1 -r recipient2you can now write-Arr recipient1 recipient2.Fixes #25