lahmatiy/open-in-editor

fix(makeArguments): No correct work if editor is emacs.

mitsuyoshi4 opened this issue · 3 comments

mitsuyoshi4 commented

@lahmatiy I encountered a bug that open-in-editor does't work correctly when editor is emacs.

Background
reference:
aws-amplify/amplify-cli#1246

Bug summary
When aws-amplify/amplify-cli apply an option of add api, an error occurred as below.

Error: Command failed: osascript -e 'tell application "Terminal" to do script "cd /Users/chris/Projects/training/nab2019-aws-workshop/aws-amplify-unicorntrivia-workshop-master; emacs --no-splash \"+1:1\" \"/Users/chris/Projects/training/nab2019-aws-workshop/amplify/backend/api/adminpanel/schema.graphql"'\"
266:268: syntax error: A “"” can’t go after this “"”. (-2740)

I researched this bug and I found out that open-in-editor's escaping logic is wrong.

My proposal

To fix this bug, change lib/open.js as below.

diff --git a/lib/open.js b/lib/open.js
index 4d08053..50df999 100644
--- a/lib/open.js
+++ b/lib/open.js
@@ -33,7 +33,7 @@ function makeArguments(filename, settings) {
     //   {filename}:1:2
     //   => "filename:1:2"
     //
-    .replace(/\{filename\}(\S*)/, function(m, rest) {
+    .replace(/\{filename\}([^\\'"\s]*)/, function(m, rest) {
       return quote(info.filename + rest, settings.escapeQuotes);
     });
 }

Pull request
#16

Please consider that, I hope my pull request will be help to open-in-editor.

Thank you and best regard.

lahmatiy commented

@mitsuyoshi4 Thank you! I need a time to validate changes, since it affects all editors not emacs only.

  • 👍1
kaustavghosh06 commented

@lahmatiy Would it be possible to merge this PR?

kaustavghosh06 commented

Hey guys, would it be possible to merge this PR sometime soon? This has been pending review for a merge since the last 2 months.

  • 👍1