Skip to content

Commit

Permalink
Fix false “undefined name” message caused by cell magic (#1007)
Browse files Browse the repository at this point in the history
* Fix cell magic is run in a different scope

* Apply lint

* Update CHANGELOG

* Fix regex

* Resolve traitlets type warnings, lint, remove `six`

---------

Co-authored-by: krassowski <5832902+krassowski@users.noreply.github.com>
  • Loading branch information
i-aki-y and krassowski committed Nov 25, 2023
1 parent ebcb064 commit 82bf833
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 4 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1101,8 +1101,14 @@ Requires JupyterLab `>=3.6.0,<4.0.0a0` and Python 3.8 or newer.
### `jupyter-lsp 0.6.0b0`

- features

- starts language servers on demand
- accepts configuration via Jupyter config system (traitlets) and python
`entry_point`s
- autodetects language servers for bash, CSS, LESS, SASS, Dockerfile, YAML, JS,
TypeScript, JSX, TSX, JSON, YAML

- bugfixes
- fix issue that variables declared in cell magics(%%time, %%capture) are masked(
[#635](https://github.com/jupyter-lsp/jupyterlab-lsp/issues/635)
)
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ describe('Default IPython overrides', () => {
expect(reverse).toBe(cellMagicWithArgs);
});

it('some cell magic commands are unwrapped', () => {
const cellMagicToUnwrap = '%%capture --no-display\ntext';
let override = cellMagicsMap.overrideFor(cellMagicToUnwrap)!;
expect(override).toBe(
'# START_CELL_MAGIC("capture", " --no-display")\ntext\n# END_CELL_MAGIC'
);

let reverse = cellMagicsMap.reverse.overrideFor(override);
expect(reverse).toBe(cellMagicToUnwrap);
});

it('escapes docstrings properly', () => {
let override = cellMagicsMap.overrideFor(CELL_MAGIC_WITH_DOCSTRINGS)!;
expect(override).toBe(
Expand Down
30 changes: 26 additions & 4 deletions packages/jupyterlab-lsp/src/transclusions/ipython/overrides.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ function emptyOrEscaped(x: string) {
}
}

const MAGICS_TO_UNWRAP = ['time', 'capture'];

function unwrapCellMagic(name: string, firstLine: string, content: string) {
return `# START_CELL_MAGIC("${name}", "${firstLine}")
${content}
# END_CELL_MAGIC`;
}

/**
* Line magics do not have to start with the new line, for example:
* x = !ls
Expand Down Expand Up @@ -131,13 +139,27 @@ export let overrides: IScopedCodeOverride[] = [
firstLine = firstLine.slice(0, -1);
}
content = content.replace(/"""/g, '\\"\\"\\"');
return `get_ipython().run_cell_magic("${name}", "${firstLine}", """${content}""")`;

let replaced: string;
if (MAGICS_TO_UNWRAP.includes(name)) {
replaced = unwrapCellMagic(name, firstLine, content);
} else {
replaced = `get_ipython().run_cell_magic("${name}", "${firstLine}", """${content}""")`;
}
return replaced;
},
scope: 'cell',
reverse: {
pattern:
'^get_ipython\\(\\).run_cell_magic\\("(.*?)", "(.*?)", """([\\s\\S]*)"""\\)',
replacement: (match, name, line, content) => {
pattern: '^get_ipython[\\s\\S]*|^# START_CELL_MAGIC[\\s\\S]*',
replacement: code => {
const regCellMagic = RegExp(
'^get_ipython\\(\\).run_cell_magic\\("(.*?)", "(.*?)", """([\\s\\S]*)"""\\)'
);
const regUnwrapped = RegExp(
'^# START_CELL_MAGIC\\("(.*?)", "(.*?)"\\)\\n([\\s\\S]*)\\n# END_CELL_MAGIC$'
);
let m = code.match(regCellMagic) || code.match(regUnwrapped);
let [name, line, content] = m?.slice(1, 4) || ['', '', ''];
content = content.replace(/\\"\\"\\"/g, '"""');
line = unescape(line);
return `%%${name}${line}\n${content}`;
Expand Down

0 comments on commit 82bf833

Please sign in to comment.