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

Add an option to allow disabling ampersand escaping #74

Closed

Conversation

MarshallOfSound
Copy link

We've found when launching files with a "&" in their filename this escaping breaks the launch. (Windows throws a could not find file error). This PR just adds an option to allow disabling the escaping 👍

@sindresorhus
Copy link
Owner

Can't we just handle this automatically, so it only escapes on URLs? I prefer no options whenever possible. Every option added is a lot of wasted time for consumer needing to understand the implications and whether they need it.

@MarshallOfSound
Copy link
Author

@sindresorhus I looked and asked around and I can't figure out for sure if it's just URLs that need the "&" escaped. Are you OK with a basic "if it's an http:// or https:// string, escape, else, don't" style logic?

@kevva
Copy link

kevva commented Oct 6, 2017

What about skipping the escaping and wrapping it in double quotes instead? Maybe that will be the same as escaping them though. I don't have access to Windows so can't test.

@baoti
Copy link

baoti commented Jul 6, 2018

@MarshallOfSound Can you provide a filename to test?

I create a file named a&b.docx, And I can open it using open('G:\\test-cmd-start\\a&b.docx')

@baoti
Copy link

baoti commented Jul 10, 2018

@MarshallOfSound @sindresorhus I had test these targets on Windows 7:

target escape mode open result
G:\&b.docx no escape FAIL
G:\&b.docx escape by ^ SUCCESS
G:\a&b.docx no escape FAIL
G:\a&b.docx escape by ^ SUCCESS
G:\a&b c.docx no escape SUCCESS
G:\a&b c.docx escape by ^ FAIL
G:\a&b c.docx using " " wraping space only FAIL
G:\a&b c.docx using " " wraping target FAIL
G:\a&b\c.docx no escape FAIL
G:\a&b\c.docx escape by ^ SUCCESS
G:\a b\c.docx no escape SUCCESS
G:\a b\c.docx escape by ^ FAIL
https://bing.com/search?q=a&b^c no escape FAIL (open wrong url)
https://bing.com/search?q=a&b^c escape by ^ SUCCESS (open url q=a&b^c)
https://bing.com/search?q=a&b^c using " " wraping target FAIL
https://bing.com/search?q=a b^c no escape SUCCESS (open url q=a%20b^c)
https://bing.com/search?q=a b^c escape by ^ FAIL (open wrong url)
https://bing.com/search?q=a b^c using " " wraping target FAIL
file:///G:/a&b.docx no escape FAIL
file:///G:/a&b.docx escape by ^ SUCCESS
file:///G:/a&b.docx using " " wraping target FAIL
file:///G:/a b.docx no escape SUCCESS
file:///G:/a b.docx escape by ^ FAIL
file:///G:/a b.docx using " " wraping target FAIL

Finally, I got 3 conclusions:

  • If contains space, DO NOT escape;
  • otherwise, DO escape.
  • No matter target is URL, or file path.

baoti added a commit to baoti/opn that referenced this pull request Jul 10, 2018
@sindresorhus
Copy link
Owner

Closing this in favor of #98

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.

None yet

4 participants