Handle stderr when using edit or config command #622
Conversation
astorije
added
Type: Bug
|
Not blocking v2.0.0 release, |
| child.spawn( | ||
| process.env.EDITOR || "vi", | ||
| var child_spawn = child.spawn( | ||
| process.env.EDITOR || "vi" || "nano", |
There was a problem hiding this comment.
Your || "nano" addition doesn't make much sense, as the vi fallback is for the EDITOR being undefined.
There was a problem hiding this comment.
In case vi isn't present, we should try nano. @astorije's idea
EDIT: I see; EDITOR is a variable in this case - vi won't be undefined in javascript. Fixing.
| process.env.EDITOR || "vi", | ||
| [Helper.CONFIG_PATH], | ||
| {stdio: "inherit"} | ||
| ); | ||
| child_spawn.on("error", function() { | ||
| log.error("$EDITOR is not set, and vi was not found."); |
There was a problem hiding this comment.
What I would do is print Helper.CONFIG_PATH here so that users can easily edit the file in their favourite editor.
| process.env.EDITOR || "vi", | ||
| [Helper.getUserConfigPath(name)], | ||
| {stdio: "inherit"} | ||
| ); | ||
| child_spawn.on("error", function() { | ||
| log.error("Unable to open " + Helper.CONFIG_PATH + ". $EDITOR is not set, and vi was not found."); |
There was a problem hiding this comment.
Changed to Helper.getUserConfigPath(name)
astorije
removed
Meta: Do Not Merge
|
@xPaw, against the idea of falling back to nano if |
|
@astorije I don't know which editor is more commonly preinstalled, but I am fine with changing |
|
I was actually suggesting both, but maybe a bad idea. I don't care which one really, as long as the default is sensible. Let's go with that for now, changing default editor would be a different PR anyway, sorry for shifting focus. |
astorije
changed the title
Handle stderr when using edit or config command, fixes thelounge#164
Fixes #164.