open-in-editor icon indicating copy to clipboard operation
open-in-editor copied to clipboard

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

Open mitsuyoshi4 opened this issue 6 years ago • 3 comments

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

Background reference: https://github.com/aws-amplify/amplify-cli/issues/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 https://github.com/lahmatiy/open-in-editor/pull/16

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

Thank you and best regard.

mitsuyoshi4 avatar Apr 15 '19 05:04 mitsuyoshi4

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

lahmatiy avatar Apr 16 '19 08:04 lahmatiy

@lahmatiy Would it be possible to merge this PR?

kaustavghosh06 avatar Jun 06 '19 17:06 kaustavghosh06

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.

kaustavghosh06 avatar Jun 27 '19 17:06 kaustavghosh06