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

cmd task: Strip quotes from token on Windows #145

Merged
merged 2 commits into from
May 7, 2023

Conversation

ameily
Copy link
Contributor

@ameily ameily commented May 4, 2023

I found an issue related to #16 where, on Windows, a quoted token is being passed to the command still wrapped in quotes. This only happens on Windows because the shelx.split(posix=False) preserves them. You can verify this issue on Windows with the poethepoety test-quick task, which passes a quotes string to pytest and fails because the marker contains an invalid quote character:

Poe => pytest --cov=poethepoet -m "not slow"
# ... cut ...
ERROR: Wrong expression passed to '-m': "not slow": at column 1: unexpected character ""

I initially was seeing this with a call to cspell and have reproduced it with a small script / task.

# args.py
import sys
print(sys.argv)
# pyproject.toml
[tool.poe.tasks.args]
cmd = "python args.py \"hello world\" \"some/glob/**/*.py\""

The results of running the args task show that, on Windows, the quotes are preserved when they should be stripped.

$ poetry poe args
Poe => python args.py "hello world" "some/glob/**/*.py"
['args.py', '"hello world"', '"some/glob/**/*.py"']

I tried using some of the examples from #16 but I couldn't get them to work in my case, either the glob was being expanded by poe or the quote characters were being passed to the subprocess.

@ameily
Copy link
Contributor Author

ameily commented May 4, 2023

It looks like the Windows tests are failing because they expect the quoted behavior in stdout. If you're ok with my change, I'll update the tests and get them passing.

@nat-n
Copy link
Owner

nat-n commented May 4, 2023

Hi @ameily, thanks for working on this! If you fix the tests I'd be happy to merge it.

@ameily
Copy link
Contributor Author

ameily commented May 4, 2023

This should be good to go. Thanks for maintaining poe and being responsive! I useit for several Windows and Linux projects.

@nat-n nat-n merged commit 2816cf2 into nat-n:main May 7, 2023
github-actions bot pushed a commit that referenced this pull request May 7, 2023
This change fixes an issue whereby quoted tokens in cmds were passed to the executable without stripping the quotes. Related to #16 2816cf2
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.

2 participants