search

Jayich-Lab / tray-launcher

A launcher for Windows that resides in the taskbar for managing .bat scripts.
MIT License
7 stars 1 forks source link

Tray launcher does not support user folder paths with spaces #42

Closed fanmingyu212 closed 2 years ago

fanmingyu212 commented 2 years ago

If the user folder path has a space in it, it cannot run scripts and crashes when the user tries to run a script. One solution may be add " before and after the script_path_str in the constructor of ChildScript.

danhuaxu commented 2 years ago

Ha, okay. I thought space is also not allowed for folders as it is not for file names.

Just a note: if the tray launcher crashes, it won't terminate scripts that are currently running. So you might need to terminate them manually unfortunately. I haven't found a good way to do clean-up upon abnormal exit. Any ideas?

fanmingyu212 commented 2 years ago

I think spaces are allowed for both folders and filenames. A good practice may be avoiding using spaces, but that is not always done (e.g. "C:\Program Files" have a space in it.

danhuaxu commented 2 years ago

I see... What you suggested should work. Thanks.

danhuaxu commented 2 years ago

@fanmingyu212 Hey Mingyu, sorry it takes me so long to get back to this!

Do you remember what command did you run that crushed the launcher? I tried to load/run a script from a folder whose name contains a space, and I was getting error messages saying that the path is invalid (I suspect that this is due to argparse module uses whitespace as a delimiter, so the path got truncated into two elements in the argument list) - the launcher didn't crash.

Running such a script from GUI hasn't shown any problems, so this issue seems to be only related to CLI.

I will be resolving hopefully all the comments tonight.

fanmingyu212 commented 2 years ago

I think the problem I had was that the user folder path contains a space. That means the tray launcher path in the user folder also contains a space. It crashed when I tried to run a script using the GUI (not related to the CLI at all).

danhuaxu commented 2 years ago

I see.. Thanks for the info. This is very screwed up.

fanmingyu212 commented 2 years ago

No worry. I think it is just missing quotes in the POpen command that runs the script. I implemented that fix locally (see the first comment I have above). If you want, I can create a PR for it.

danhuaxu commented 2 years ago

Ah, that’s what you meant! Got it.

Yes a PR will be very helpful. Thank you!