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

Improve how trogon is calling the command with arguments #107

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bigbirdcode
Copy link

This PR tries to resolve several problems. All the changes are small and connected, that is why I opened just one PR.

Please review it commit by commit.

Fixes:

#104 - In run_command.py file the UserCommandData.to_cli_args() now contains string conversion.
I have not added conversion to _to_cli_args() as it was already too complex.

#105 - shlex replaced to oslex, for the need of Windows users.
I have added to pyproject.toml.
TODO: poetry.lock was not updated, I don't have tooling for that.

#106 - To run well and wait for the output on Windows I have added sys.exit(subprocess.call(..., shell=True)) as the pair of os.execvp() call.
Note: shell=True was needed for me.

#81 , #70 , #67 , #65 - I try to fix all these. In the file detect_run_string.py the function detect_run_string() returns a short version of the command, while for execution we need the full version. So I have added a function exact_run_commands(). I have also modified how the command is called in trogon.py and replaced os.execvp() to os.execv().

Tested:

  • Windows, from venv with and without -m, outside of a venv with direct path to Python.exe, and with installed app
  • Linux, from venv with and without -m, outside of a venv with direct path to python3

I have not tested with pipx or such cases. Please tell me of more modification is needed.

@bigbirdcode bigbirdcode changed the title Improve cli args Improve how trogon is calling the command with arguments Feb 1, 2025
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.

1 participant