Correct rpy2 %Rdevice magic integration (#646)

and prevent breaking of other line magics which also happen to start with R.
jupyter-lsp · Jul 16, 2021 · e4cecfc · e4cecfc
1 parent bba7fd1
commit e4cecfc
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,12 @@
 ## Changelog
 
+### `@krassowski/jupyterlab-lsp 3.8.1` (unreleased)
+
+- bug fixes:
+  - `%Rdevice` magic is now properly overridden and won't be extracted to R code [(#646)]
+
+[#646]: https://github.com/krassowski/jupyterlab-lsp/pull/646
+
 ### `@krassowski/jupyterlab-lsp 3.8.0` (2021-07-04)
 
 - improvements:

diff --git a/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/extractors.spec.ts b/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/extractors.spec.ts
@@ -34,6 +34,14 @@ describe('IPython rpy2 extractors', () => {
   });
 
   describe('%R line magic', () => {
+    it('should not extract parts of non-code commands', () => {
+      let code = wrap_in_python_lines('%Rdevice svg');
+      let { cell_code_kept, foreign_document_map } = extract(code);
+
+      expect(cell_code_kept).to.equal(code);
+      expect(foreign_document_map.size).to.equal(0);
+    });
+
     it('correctly gives ranges in source', () => {
       let code = '%R ggplot()';
       let { foreign_document_map } = extract(code);

diff --git a/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/extractors.ts b/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/extractors.ts
@@ -53,7 +53,8 @@ export let foreign_code_extractors: IForeignCodeExtractorsRegistry = {
       language: 'r',
       // it is very important to not include the space which will be trimmed in the capture group,
       // otherwise the offset will be off by one and the R language server will crash
-      pattern: '(?:^|\n)%R' + rpy2_args_pattern(RPY2_MAX_ARGS) + ' ?(.*)?\n?',
+      pattern:
+        '(?:^|\n)%R' + rpy2_args_pattern(RPY2_MAX_ARGS) + '(?: (.*))?(?:\n|$)',
       foreign_capture_groups: [RPY2_MAX_ARGS * 2 + 1],
       foreign_replacer: create_rpy_code_extractor(true),
       extract_arguments: rpy2_args,

diff --git a/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.spec.ts b/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.spec.ts
@@ -34,6 +34,23 @@ describe('rpy2 IPython overrides', () => {
     let line_magics = new ReversibleOverridesMap(
       overrides.filter(override => override.scope == 'line')
     );
+
+    it('works with other Rdevice', () => {
+      let line = '%Rdevice svg';
+      let override = line_magics.override_for(line);
+      expect(override).to.equal(
+        'rpy2.ipython.rmagic.RMagics.Rdevice(" svg", "")'
+      );
+      let reverse = line_magics.reverse.override_for(override);
+      expect(reverse).to.equal(line);
+    });
+
+    it('does not overwrite non-rpy2 magics', () => {
+      let line = '%RestMagic';
+      let override = line_magics.override_for(line);
+      expect(override).to.equal(null);
+    });
+
     it('works with the short form arguments, inputs and outputs', () => {
       let line = '%R -i x';
       let override = line_magics.override_for(line);

diff --git a/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.ts b/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/overrides.ts
@@ -13,12 +13,17 @@ export let overrides: IScopedCodeOverride[] = [
   {
     // support up to 10 arguments
     pattern:
-      LINE_MAGIC_PREFIX + '%R' + rpy2_args_pattern(RPY2_MAX_ARGS) + '(.*)(\n)?',
+      LINE_MAGIC_PREFIX +
+      '%R' +
+      rpy2_args_pattern(RPY2_MAX_ARGS) +
+      '( .*)?(\n|$)',
     replacement: (match, prefix, ...args) => {
       let r = parse_r_args(args, -4);
       // note: only supports assignment or -o/--output, not both
       // TODO assignment like in x = %R 1 should be distinguished from -o
-      return `${prefix}${r.outputs}rpy2.ipython.rmagic.RMagics.R("${r.content}", "${r.others}"${r.inputs})`;
+      return `${prefix}${r.outputs}rpy2.ipython.rmagic.RMagics.R("${
+        r.content || ''
+      }", "${r.others}"${r.inputs})`;
     },
     scope: 'line',
     reverse: {
@@ -47,5 +52,20 @@ export let overrides: IScopedCodeOverride[] = [
       },
       scope: 'cell'
     }
+  },
+  {
+    pattern: LINE_MAGIC_PREFIX + '%Rdevice( .*)?(\n|$)',
+    replacement: (match, prefix, ...args) => {
+      return `${prefix}rpy2.ipython.rmagic.RMagics.Rdevice("${args[0]}", "")`;
+    },
+    scope: 'line',
+    reverse: {
+      pattern: rpy2_reverse_pattern('"', false, 'Rdevice'),
+      replacement: (match, ...args) => {
+        let r = rpy2_reverse_replacement(match, ...args);
+        return '%Rdevice' + r.input + r.output + r.other + r.contents;
+      },
+      scope: 'line'
+    }
   }
 ];
diff --git a/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/rpy2.ts b/packages/jupyterlab-lsp/src/transclusions/ipython-rpy2/rpy2.ts
@@ -46,11 +46,17 @@ export function parse_r_args(args: string[], content_position: number) {
   };
 }
 
-export function rpy2_reverse_pattern(quote = '"', multi_line = false): string {
+export function rpy2_reverse_pattern(
+  quote = '"',
+  multi_line = false,
+  magic = 'R'
+): string {
   return (
     '(\\S+)?' +
     '(?:, (\\S+))?'.repeat(9) +
-    '( = )?rpy2\\.ipython\\.rmagic\\.RMagics.R\\(' +
+    '( = )?rpy2\\.ipython\\.rmagic\\.RMagics.' +
+    magic +
+    '\\(' +
     quote +
     (multi_line ? '([\\s\\S]*)' : '(.*?)') +
     quote +