Merge pull request #293 from krassowski/improve-magic-false-positives

Alleviate too aggressive line magics substitution #281

Author: krassowski

CHANGELOG.md

@@ -1,16 +1,44 @@
 ## CHANGELOG
 
-### `@krassowski/jupyterlab-lsp 1.0.1` (unreleased)
+### `@krassowski/jupyterlab-lsp 1.1.0` (unreleased)
+
+- features
+
+  - language servers can now be configured from the Advanced Settings Editor ([#245][])
 
 - bug fixes
 
   - fixes currently-highlighted token in dark editor themes against light lab theme
     (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 ([#293][])
 
 [#195]: https://github.com/krassowski/jupyterlab-lsp/issues/195
 [#261]: https://github.com/krassowski/jupyterlab-lsp/issues/261
+[#293]: https://github.com/krassowski/jupyterlab-lsp/pull/293
+
+### `@krassowski/jupyterlab-lsp 0.9.0` (unreleased)
+
+- features
+
+  - language servers can now be configured from the Advanced Settings Editor ([#245][])
+
+- bug fixes
+
+  - handles document open and change operation ordering more predictably ([#284][])
+
+### `lsp-ws-connection 0.5.0` (unreleased)
+
+- features
+
+  - language servers can now be configured from the Advanced Settings Editor ([#245][])
+
+- bug fixes
+
+  - handles document open and change operation ordering more predictably ([#284][])
+
+[#245]: https://github.com/krassowski/jupyterlab-lsp/pull/245
 [#284]: https://github.com/krassowski/jupyterlab-lsp/pull/284
 
 ### `@krassowski/jupyterlab-lsp 1.0.0` (2020-03-14)

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

@@ -104,6 +104,18 @@ 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 +134,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: