javascript-studio/studio-changes

changes fails in a dirty state when $EDITOR is not set and vim is not installed

m90 opened this issue · 5 comments

m90 commented

We just adopted @studio/changes and ❤️ it very much.

We found one issue though: when $EDITOR is not set and vim is not installed, changes will fail with:

Error: spawn vim ENOENT

and a dirty working directory. This is due to the fact that changes.abort in the callback of editor will never be called. Unfortunately it is also not possible to try/catch that block as it is spawning a child process and there is nothing to catch.

This would be fixable if: https://github.com/substack/node-editor/pull/13 got merged but I doubt that will ever land.

Maybe an acceptable solution would be exiting early if $EDITOR is not set at all and prompting the user to set it?

It should be fixed in node-editor. It would be a breaking change to fail if $EDITOR is not set, and really, it should just work with some default, like it currently does. I made a suggestion on your pull request on how to implement it as a fix that wouldn't require any work for node-editor consumers. Let's see if you can get this merged first.

m90 commented

Good point about the breaking nature of exiting early. I changed the PR in node-editor according to your suggestion. If that ever gets merged, I will open a PR here with an update.

Cool, thank you. Should your suggestion get merged, there is nothing to be done here, I think. Let's keep this issue open in case other come across the same problem. We can close it once we have a solution.

I now maintain a fork of editor under Studio Editor with @m90's PR applied in javascript-studio/studio-editor@58f4bb4.

This project now dependes on @studio/changes instead of editor which should fix this issue. If it doesn't, we're now able to make changes.

Released in v1.4.2.