-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Weird behaviour when opening config on Windows #1529
Comments
@Stanzilla you are opening the config using the Bash, that's why it only echoes the text as |
Looks like #1510 reverted the previous windows fix for this. |
@jonathandelgado it wasn't really a fix, it just opened the directory instead of directly opening the editor. I am trying to find a better fix as we speak. |
@stefanivic it does that with cmd as well |
Can the process of trying to open it be done in background though? Also the echo, currently looks like a debug print left there by accident, if you ask me :D |
@Stanzilla We got this merged : 36f96ab So now, it will check which shell you are runing and open with coresponding option. The |
Related: #901 |
Of course, that doesn't work with powershell, which has a different syntax for environment variables ($Env:variableName). Also, notepad?? it can't make sense of Unix style line endings, which is the default for the preferences file. It might make sense to look that up from the configuration (or environment) |
@MartyGentillon the line-ending issue was fixed with #1713 |
My main gripe with this was the echo anyway, so if we could make the settings open without any echo or new tab spawning, that would be great |
Yes I think that would be better, use something like https://github.com/sindresorhus/opn ? |
I don't think we really need any addition library to solve this, you can remove the echo frome the code and still have it working. But to be honest i think the best option would be to replace the echo and have a notification pop up. |
I think opn is a good solution. With this, editing config directly uses an OS feature and do not depend on a shell/term feature. |
@albinekb The line endings are only one of very many reasons to dislike the choice of notepad when other editors are available. Roughly speaking, the editor should be controllable either though the editor environment variable or configuration. As for using a library rather than the shell, vastly simplifies the logic and error proofs the solution. Suddenly there is no need for a "which shell are you running" guessing game to decide how to include the environment variable. |
Alternately, Gordon's solution is interesting: |
Another reason to launch editor with node/lib: #417 |
One other advantage of the library approach: it works even when you have sshed into another machine. |
Is this even remotely correct? It works fine on WIndows Stanzilla@12ffb17 (first timer here) |
Ok guys, I found a solution: https://github.com/electron/electron/blob/master/docs/api/shell.md#shellopenitemfullpath We don't need a lib, Electron will do it for us :) |
Another reason to launch native editor: https://zeit-community.slack.com/archives/C1TMVKPFH/p1493735061557196 |
Not to mention that not all systems come with nano.... |
Issue
When opening the config, a new tab is opened and gets a lot of echos, is this the correct behaviour? From the text it is looking like it's failing a few times but notepad is actually opened.
Should opening the config even spawn a new tab at all?
The text was updated successfully, but these errors were encountered: