Alleviate too aggressive line magics substitution #281

Author: krassowski

CHANGELOG.md

@@ -8,6 +8,7 @@
     (and vice versa) ([#195][])
   - restores sorting order-indicating caret icons in diagnostics panel table ([#261][])
   - handles document open and change operation ordering more predictably ([#284][])
+  - fixes some pyflakes issues caused by line magics substitution
 
 [#195]: https://github.com/krassowski/jupyterlab-lsp/issues/195
 [#261]: https://github.com/krassowski/jupyterlab-lsp/issues/261

packages/jupyterlab-lsp/src/magics/defaults.spec.ts

@@ -104,6 +104,20 @@ describe('Default IPython overrides', () => {
       expect(reverse).to.equal(LINE_MAGIC_WITH_SPACE);
     });
 
+    it('overrides x =%ls', () => {
+      // this is a corner-case as described in
+      // https://github.com/krassowski/jupyterlab-lsp/issues/281#issuecomment-645286076
+      let override = line_magics_map.override_for('x =%ls');
+      expect(override).to.equal(
+        'x =get_ipython().run_line_magic("ls", "")'
+      );
+    });
+
+    it('does not override line-magic-like constructs', () => {
+      let override = line_magics_map.override_for('list("%")');
+      expect(override).to.equal(null);
+    });
+
     it('escapes arguments', () => {
       const line_magic_with_args = '%MAGIC "arg"';
       let override = line_magics_map.override_for(line_magic_with_args);
@@ -122,5 +136,10 @@ describe('Default IPython overrides', () => {
       let reverse = line_magics_map.reverse.override_for(override);
       expect(reverse).to.equal('!ls -o');
     });
+
+    it('does not override shell-like constructs', () => {
+      let override = line_magics_map.override_for('"!ls"');
+      expect(override).to.equal(null);
+    });
   });
 });

packages/jupyterlab-lsp/src/magics/defaults.ts

@@ -38,9 +38,13 @@ export const language_specific_overrides: IOverridesRegistry = {
       //  keep the content, keep magic/command name and new line at the end
 
       // note magics do not have to start with the new line, for example x = !ls or x = %ls are both valid.
+      // x =%ls is also valid. However, percent may also appear in strings, e.g. ls('%').
+      // Hence: (^|\s|=) for shell commands (!) and line magics (%); see issue #281.
+      // This does not solve all issues, for example `list(" %ls")` still leads to:
+      // `list(" get_ipython().run_line_magic("ls")", "")`.
       {
-        pattern: '!(\\S+)(.*)(\n)?',
-        replacement: 'get_ipython().getoutput("$1$2")$3',
+        pattern: '(^|\s|=)!(\\S+)(.*)(\n)?',
+        replacement: '$1get_ipython().getoutput("$2$3")$4',
         reverse: {
           pattern: 'get_ipython\\(\\).getoutput\\("(.*?)"\\)(\n)?',
           replacement: '!$1$2'
@@ -62,11 +66,11 @@ export const language_specific_overrides: IOverridesRegistry = {
         }
       },
       {
-        pattern: '%(\\S+)(.*)(\n)?',
-        replacement: (match, name, args, line_break) => {
+        pattern: '(^|\\s|=)%(\\S+)(.*)(\n)?',
+        replacement: (match, prefix, name, args, line_break) => {
           args = empty_or_escaped(args);
           line_break = line_break || '';
-          return `get_ipython().run_line_magic("${name}", "${args}")${line_break}`;
+          return `${prefix}get_ipython().run_line_magic("${name}", "${args}")${line_break}`;
         },
         reverse: {
           pattern: