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

fix for filenames with spaces on windows #11

Merged
merged 1 commit into from
Sep 24, 2017
Merged

Conversation

reiner-dolp
Copy link
Contributor

@reiner-dolp reiner-dolp commented Sep 17, 2017

before this change, opening a path containing a space only opens an empty cmd prompt.

Only tested on Windows 10.

@Byron
Copy link
Owner

Byron commented Sep 24, 2017

Thanks a lot, and sorry for the late reply.
Even though I have no doubts this works for you, I wonder if you could verify it to work on the other cases too, or on other windows versions.

@reiner-dolp
Copy link
Contributor Author

reiner-dolp commented Sep 24, 2017

the windows start command has a unfortunate design where it takes an optional title as first argument. I think the problem is that a call start arg without an optional title is interpreted as title="" command=arg for paths without spaces and as title=arg cmd="" for paths with spaces. This patch explicitly sets the title to "" to resolve the issue.

German help output:

C:\Users\Reiner>start /?
Startet ein eigenes Fenster, um ein bestimmtes Programm oder einen Befehl
auszuführen.

START ["Titel"] [/D Pfad] [/I] [/MIN] [/MAX] [/SEPARATE | /SHARED]
      [/LOW | /NORMAL | /HIGH | /REALTIME] | /ABOVENORMAL | /BELOWNORMAL]
      [/NODE <NUMA-Knoten>] [/AFFINITY <Hex.-Affinitätsmaske>] [/WAIT] [/B]
      [Befehl/Programm]
      [Parameter]

A popular javscript library employs the same fix.

@Byron
Copy link
Owner

Byron commented Sep 24, 2017

Thanks a bunch for the explanation!
This also means that cmd (or start) parse their arguments themselves and split across spaces, as Rust does not use a shell when spawning programs which would be prone to spaces in arguments otherwise.
It’s odd, for sure 😁.

@Byron Byron merged commit 59dd0bd into Byron:master Sep 24, 2017
@Byron
Copy link
Owner

Byron commented Sep 24, 2017

I will see to get a new release out soon

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