Fix IPython shell and line magic overrides #343

Author: krassowski

packages/jupyterlab-lsp/src/transclusions/ipython/overrides.spec.ts

@@ -104,16 +104,30 @@ describe('Default IPython overrides', () => {
       expect(reverse).to.equal(LINE_MAGIC_WITH_SPACE);
     });
 
-    it('overrides x =%ls', () => {
+    it('overrides x =%ls and 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", "")');
+
+      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);
+
+      override = line_magics_map.override_for('list(" %test")');
+      expect(override).to.equal(null);
+    });
+
+    it('does not override modulo operators', () => {
+      let override = line_magics_map.override_for('3 % 2');
+      expect(override).to.equal(null);
+
+      override = line_magics_map.override_for('3%2');
+      expect(override).to.equal(null);
     });
 
     it('escapes arguments', () => {
@@ -135,9 +149,22 @@ describe('Default IPython overrides', () => {
       expect(reverse).to.equal('!ls -o');
     });
 
+    it('overrides shell commands assignments: x =!ls and x = !ls', () => {
+      let override = line_magics_map.override_for('x =!ls');
+      expect(override).to.equal('x =get_ipython().getoutput("ls")');
+
+      override = line_magics_map.override_for('x = !ls');
+      expect(override).to.equal('x = get_ipython().getoutput("ls")');
+    });
+
     it('does not override shell-like constructs', () => {
       let override = line_magics_map.override_for('"!ls"');
       expect(override).to.equal(null);
     });
+
+    it('does not override != comparisons', () => {
+      let override = line_magics_map.override_for('1 != None');
+      expect(override).to.equal(null);
+    })
   });
 });

packages/jupyterlab-lsp/src/transclusions/ipython/overrides.ts

@@ -16,20 +16,37 @@ function empty_or_escaped(x: string) {
   }
 }
 
+/**
+ * Line magics do not have to start with the new line, for example:
+ *    x = !ls
+ *    x = %ls
+ *    x =%ls
+ * are all valid.
+ *
+ * The percent may also appear in strings, e.g. ls('%').
+ *
+ * IPython allows magics on start of a line or in assignments (but only there!), thus:
+ *    x = (!ls)
+ * is invalid syntax!
+ *
+ * Therefore we can require that the match starts with either:
+ * - zero or more whitespaces right after the beginning of a line, or
+ * - variable then equals (with optional whitespaces)
+ *
+ * This will not always work: e.g.:
+ *    x['a = !ls'] = !ls
+ * is perfectly valid IPython, but regular expressions cannot help here.
+ */
+export const LINE_MAGIC_PREFIX = '^(\\s*|\\s*\\S+\\s*=\\s*)'
+
 export let overrides: IScopedCodeOverride[] = [
   /**
    * Line magics
    */
   // filter out IPython line magics and shell assignments:
   //  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|=)!(\\S+)(.*)(\n)?',
+    pattern: LINE_MAGIC_PREFIX + '!([^=\\s]+)(.*)(\n)?',
     replacement: '$1get_ipython().getoutput("$2$3")$4',
     scope: 'line',
     reverse: {
@@ -39,7 +56,7 @@ export let overrides: IScopedCodeOverride[] = [
     }
   },
   {
-    pattern: '(^|\\s|=)%(\\S+)(.*)(\n)?',
+    pattern: LINE_MAGIC_PREFIX + '%(\\S+)(.*)(\n)?',
     replacement: (match, prefix, name, args, line_break) => {
       args = empty_or_escaped(args);
       line_break = line_break || '';