EIM-443 handle space in python path

[Hahihula]

now space in python or scoop path should no longer be problem.

[alirana01]

Question about the dual execution approach

Thanks for the fix! I have a question about the design:

With the new escaping in execute_command(), shell features like variable expansion ($HOME) and globs are now blocked. Given that:

Function	Shell overhead	Handles spaces	Shell features
execute_command()	Yes	Yes (via escaping)	No (blocked by escaping)
execute_command_direct()	No	Yes (native)	No

Question: What's the benefit of keeping execute_command() going through bash/PowerShell if escaping prevents shell features anyway? It seems like we now have shell overhead without shell benefits.

Suggestions

1. Could execute_command() just call execute_command_direct() internally? The only difference now is PATH resolution and shell environment.

2. Only create_python_venv() was updated to use execute_command_direct(). Other functions in python_utils.rs that take python paths could also have space issues:

   • detect_python_version() (line 401)
   • run_python_script() (line 786)
   • get_python_version() (line 818)

My apologies for asking clarifications in review

[Hahihula]

Question about the dual execution approach

Thanks for the fix! I have a question about the design:

With the new escaping in execute_command(), shell features like variable expansion ($HOME) and globs are now blocked. Given that:

Function Shell overhead Handles spaces Shell features
execute_command() Yes Yes (via escaping) No (blocked by escaping)
execute_command_direct() No Yes (native) No
Question: What's the benefit of keeping execute_command() going through bash/PowerShell if escaping prevents shell features anyway? It seems like we now have shell overhead without shell benefits.

Suggestions

1. Could execute_command() just call execute_command_direct() internally? The only difference now is PATH resolution and shell environment.

2. Only create_python_venv() was updated to use execute_command_direct(). Other functions in python_utils.rs that take python paths could also have space issues:

   • detect_python_version() (line 401)
   • run_python_script() (line 786)
   • get_python_version() (line 818)

My apologies for asking clarifications in review

when you are saying Shell features you meant expansion of $HOME variable, but the diferences are with lot more variables and environment settings. Generaly we should be migrating to the execute_command_direct but it is out of scope of these change to do this refactoring for whole codebase.

[alirana01]

Question about the dual execution approach

Thanks for the fix! I have a question about the design:
With the new escaping in execute_command(), shell features like variable expansion ($HOME) and globs are now blocked. Given that:
Function Shell overhead Handles spaces Shell features
execute_command() Yes Yes (via escaping) No (blocked by escaping)
execute_command_direct() No Yes (native) No
Question: What's the benefit of keeping execute_command() going through bash/PowerShell if escaping prevents shell features anyway? It seems like we now have shell overhead without shell benefits.

Suggestions

1. Could execute_command() just call execute_command_direct() internally? The only difference now is PATH resolution and shell environment.

2. Only create_python_venv() was updated to use execute_command_direct(). Other functions in python_utils.rs that take python paths could also have space issues:

   • detect_python_version() (line 401)
   • run_python_script() (line 786)
   • get_python_version() (line 818)

My apologies for asking clarifications in review

when you are saying Shell features you meant expansion of $HOME variable, but the diferences are with lot more variables and environment settings. Generaly we should be migrating to the execute_command_direct but it is out of scope of these change to do this refactoring for whole codebase.

and do we need to update the calls in these

• detect_python_version() (line 401)
• run_python_script() (line 786)
• get_python_version() (line 818)

as I believe one of the recent issue #366 could also be related to this

[Hahihula]

Question about the dual execution approach

Thanks for the fix! I have a question about the design:
With the new escaping in execute_command(), shell features like variable expansion ($HOME) and globs are now blocked. Given that:
Function Shell overhead Handles spaces Shell features
execute_command() Yes Yes (via escaping) No (blocked by escaping)
execute_command_direct() No Yes (native) No
Question: What's the benefit of keeping execute_command() going through bash/PowerShell if escaping prevents shell features anyway? It seems like we now have shell overhead without shell benefits.

Suggestions

1. Could execute_command() just call execute_command_direct() internally? The only difference now is PATH resolution and shell environment.

2. Only create_python_venv() was updated to use execute_command_direct(). Other functions in python_utils.rs that take python paths could also have space issues:

   • detect_python_version() (line 401)
   • run_python_script() (line 786)
   • get_python_version() (line 818)

My apologies for asking clarifications in review

when you are saying Shell features you meant expansion of $HOME variable, but the diferences are with lot more variables and environment settings. Generaly we should be migrating to the execute_command_direct but it is out of scope of these change to do this refactoring for whole codebase.

and do we need to update the calls in these

• detect_python_version() (line 401)
• run_python_script() (line 786)
• get_python_version() (line 818)

as I believe one of the recent issue #366 could also be related to this

You are completely right. It should be used in the whole process of the python module. I would swear i wrote it ... sorry.
Thanks for the catch.