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

Correctly handle -m and -k flags #37

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Correctly handle -m and -k flags #37

merged 1 commit into from
Nov 20, 2020

Conversation

wbolster
Copy link
Owner

@wbolster wbolster commented Oct 12, 2020

Correctly handle -m and -k flags

Add a special reader helper to read shell arguments that should go after
a short-form pytest flag.

Fixes #36.

@wbolster wbolster self-assigned this Oct 12, 2020
@wbolster
Copy link
Owner Author

@pkryger can i bug you for a review 🙏 since i'm not sure this is the best way to do this.

note that pytest does not have a --long-version for the -k and -m arguments, it seems.

Copy link
Contributor

@pkryger pkryger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is perfectly fine, and probably should be released as a fix to a non-working options.

What I'd consider would be:

  • adding a :reader to quote args, so they display nicely in transient popup,
  • probably reintroduce = in :argument, again for display purposes (otherwise the value is concatenated to the argument without a separator),
  • update python-pytest--quote-string-option to only quote unquoted arguments and properly handle '=' sign.

Add a special reader helper to read shell arguments that should go after
a short-form pytest flag.

Fixes #36.
@wbolster
Copy link
Owner Author

wbolster commented Nov 20, 2020

thanks! i updated my changes based on your suggestion, but i didn't fully understand them, and i couldn't make it work.

i ended up with a custom reader function that takes care of both the quoting and the spacing. it looks nice in the dispatcher modal (try -k, then type foo or bar), and produces a correct pytest -k 'foo or bar' command line.

(totally forgot about this for a while, until i hit the issue again recently.)

@wbolster wbolster merged commit 4a1c4c8 into master Nov 20, 2020
@wbolster wbolster deleted the fix-flag-handling branch November 20, 2020 21:06
@pkryger
Copy link
Contributor

pkryger commented Nov 22, 2020

I think this version is even nicer (and simpler!) than what I thought about.

@wbolster
Copy link
Owner Author

good to hear! thanks again for your review and ideas 🙏

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

Successfully merging this pull request may close these issues.

incorrect handling of -m and -k flags
2 participants