From 19f98dbc5057c654125bcef27471f60749945fa6 Mon Sep 17 00:00:00 2001 From: Divyansh Choudhary Date: Wed, 6 Sep 2023 15:30:14 +0530 Subject: [PATCH 1/5] Fix bugs due to pyflyby cell --- src/index.tsx | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/index.tsx b/src/index.tsx index 35daa8a..9399831 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -307,7 +307,6 @@ class PyflyByWidget extends Widget { } restoreNotebookAfterTidyImports(cellArray: any, imports: any): void { - const { cellIndex } = this._findAndSetImportCoordinates(); const cells = this._context.model.cells; for (let i = 0; i < cellArray.length; ++i) { const cell = cells.get(i); @@ -315,20 +314,19 @@ class PyflyByWidget extends Widget { cell.value.insert(0, cellArray[i].text.trim()); } const joined_imports = imports.join('\n').trim(); - if (cells.get(0).value.text.length === 0) { - cells.get(0).value.insert(0, joined_imports); - } else { - const cell = this._context.model.contentFactory.createCodeCell({ - cell: { - source: joined_imports, - cell_type: 'code', - metadata: { - trusted: true - } - } - }); - cells.insert(cellIndex, cell); - } + const { cellIndex } = this._findAndSetImportCoordinates(); + cells + .get(cellIndex) + .value.remove(0, cells.get(cellIndex).value.text.length); + + // `joined_imports` contains all the imports in the notebook so it is safe + // to wrap it under PYFLYBY comments and insert into the pyflyby cell directly + cells + .get(cellIndex) + .value.insert( + 0, + `${PYFLYBY_START_MSG}${joined_imports}\n${PYFLYBY_END_MSG}` + ); } _fastStringHash(str: string) { From f071b18ab7b236012834a2a970e07afee33e6286 Mon Sep 17 00:00:00 2001 From: Divyansh Choudhary Date: Wed, 6 Sep 2023 17:29:09 +0530 Subject: [PATCH 2/5] Add changelog and update version --- CHANGELOG.md | 9 +++++++++ package.json | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a3898..0e3f58f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,15 @@ Support to run `tidy-imports` on a notebook. - **Breaking**: Ported to JupyterLab 4.x +## [4.3.2](https://github.com/deshaw/jupyterlab-pyflyby/compare/v4.3.1...v4.3.2) (UNRELEASED) + +### Fixed + +- Fixed a bug where multiple clicks of the TidyImportsButton caused the notebook to be modified in unexpected ways. Some of these were - + - Outputs of cells being mismatched when reconstructing the notebook. + - Addition of random cells above and below the pyflyby tagged cell. + - Mutliple clicks of the TidyImportButton would keep adding extra lines in the pyflyby cell. + ## [4.3.1](https://github.com/deshaw/jupyterlab-pyflyby/compare/v4.3.0...v4.3.1) (2023-08-10) ### Fixed diff --git a/package.json b/package.json index 5aff6a8..4a2827d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@deshaw/jupyterlab-pyflyby", - "version": "4.3.1", + "version": "4.3.2", "description": "A labextension to integrate pyflyby with notebooks", "keywords": [ "jupyter", From 92ca268dc1963874c87c037ff0dcf4097a03bae1 Mon Sep 17 00:00:00 2001 From: Divyansh Choudhary Date: Tue, 3 Oct 2023 12:24:34 +0530 Subject: [PATCH 3/5] Address review comments --- src/cellUtils.ts | 32 +++++++++++++++++++++- src/index.tsx | 70 +++++++++++++++++++++++++++++++++++++----------- 2 files changed, 86 insertions(+), 16 deletions(-) diff --git a/src/cellUtils.ts b/src/cellUtils.ts index 8b610e1..c8bbf03 100644 --- a/src/cellUtils.ts +++ b/src/cellUtils.ts @@ -1,7 +1,7 @@ import { toArray } from '@lumino/algorithm'; import { MultilineString } from '@jupyterlab/nbformat'; import { ICellModel } from '@jupyterlab/cells'; -import { PYFLYBY_END_MSG } from './constants'; +import { PYFLYBY_END_MSG, PYFLYBY_START_MSG } from './constants'; // FIXME: There's got to be a better Typescript solution // for distinguishing between members of a union type at runtime. @@ -119,3 +119,33 @@ export const findLinePos = (cell: ICellModel): number => { // These imports will be moved to next cell return -1; }; + +/** + * Find the line number which contains the PYFLYBY_END_MSG. + * Returns -1 if the PYFLYBY_END_MSG doesn't exist. + * + * @param cell - a cell model + */ +export const extractCodeFromPyflybyCell = (cell: ICellModel): string => { + const lines: string[] = normalizeMultilineString(cell.toJSON().source); + + let stIdx = -1, + enIdx = -1; + for (let i = 0; i < lines.length; ++i) { + if (lines[i] === PYFLYBY_START_MSG.trim()) { + stIdx = i; + } + if (lines[i] === PYFLYBY_END_MSG.trim()) { + enIdx = i; + } + } + + // we splice it twice to remove the pyflyby messages + const imports: string = lines + .splice(stIdx, enIdx - stIdx + 1) + .splice(stIdx + 1, enIdx - stIdx - 1) + .join(); + const remainingCode: string = lines.join(''); + console.log(imports, remainingCode); + return remainingCode; +}; diff --git a/src/index.tsx b/src/index.tsx index 9399831..45e3eae 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -19,7 +19,11 @@ // Lumino imports import { toArray, ArrayExt } from '@lumino/algorithm'; -import { JSONValue, JSONObject } from '@lumino/coreutils'; +import { + JSONValue, + JSONObject, + ReadonlyPartialJSONValue +} from '@lumino/coreutils'; import { Widget, Panel } from '@lumino/widgets'; // Jupyterlab imports @@ -45,7 +49,7 @@ import { debug } from 'debug'; import React from 'react'; // relative imports -import { findCell, findLinePos } from './cellUtils'; +import { extractCodeFromPyflybyCell, findCell, findLinePos } from './cellUtils'; import { PYFLYBY_CELL_TAG, PYFLYBY_START_MSG, @@ -313,20 +317,56 @@ class PyflyByWidget extends Widget { cell.value.remove(0, cell.value.text.length); cell.value.insert(0, cellArray[i].text.trim()); } - const joined_imports = imports.join('\n').trim(); + const joinedImports = imports.trim(); const { cellIndex } = this._findAndSetImportCoordinates(); - cells - .get(cellIndex) - .value.remove(0, cells.get(cellIndex).value.text.length); - - // `joined_imports` contains all the imports in the notebook so it is safe - // to wrap it under PYFLYBY comments and insert into the pyflyby cell directly - cells - .get(cellIndex) - .value.insert( - 0, - `${PYFLYBY_START_MSG}${joined_imports}\n${PYFLYBY_END_MSG}` - ); + + const remainingCodeInPyflybyCell = extractCodeFromPyflybyCell( + cells.get(cellIndex) + ); + + if (remainingCodeInPyflybyCell !== '') { + // Remove import statements from the current pyflyby cell + cells + .get(cellIndex) + .value.remove(0, cells.get(cellIndex).value.text.length); + + cells.get(cellIndex).value.insert(0, remainingCodeInPyflybyCell); + + // remove the pyflyby cell tag from the current cell, we'll create another pyflyby cell + const tags: string[] = cells + .get(cellIndex) + .metadata.get('tags') as string[]; + tags.splice(tags.indexOf(PYFLYBY_CELL_TAG)); + cells + .get(cellIndex) + .metadata.set('tags', tags as ReadonlyPartialJSONValue); + + // Create a new pyflyby cell and insert it at the top + const cell = this._context.model.contentFactory.createCodeCell({ + cell: { + source: [PYFLYBY_START_MSG, joinedImports, PYFLYBY_END_MSG].join( + '\n' + ), + cell_type: 'code', + metadata: { + trusted: true, + tags: [PYFLYBY_CELL_TAG] + } + } + }); + + cells.insert(0, cell); + } else { + cells + .get(cellIndex) + .value.remove(0, cells.get(cellIndex).value.text.length); + cells + .get(cellIndex) + .value.insert( + 0, + [PYFLYBY_START_MSG, joinedImports, PYFLYBY_END_MSG].join('\n') + ); + } } _fastStringHash(str: string) { From 581ad7d518c5dab44ea557c0a505a1190469882c Mon Sep 17 00:00:00 2001 From: Divyansh Choudhary Date: Tue, 3 Oct 2023 12:27:09 +0530 Subject: [PATCH 4/5] Fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e3f58f..63e2459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Support to run `tidy-imports` on a notebook. - Fixed a bug where multiple clicks of the TidyImportsButton caused the notebook to be modified in unexpected ways. Some of these were - - Outputs of cells being mismatched when reconstructing the notebook. - Addition of random cells above and below the pyflyby tagged cell. - - Mutliple clicks of the TidyImportButton would keep adding extra lines in the pyflyby cell. + - Mutliple clicks of the TidyImportsButton would keep adding extra lines in the pyflyby cell. ## [4.3.1](https://github.com/deshaw/jupyterlab-pyflyby/compare/v4.3.0...v4.3.1) (2023-08-10) From c009d9b20c5b4003baf49f2c8dd41a535a2a426f Mon Sep 17 00:00:00 2001 From: Divyansh Choudhary Date: Tue, 3 Oct 2023 12:39:14 +0530 Subject: [PATCH 5/5] Fix function documentation --- src/cellUtils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cellUtils.ts b/src/cellUtils.ts index c8bbf03..6ca2eb8 100644 --- a/src/cellUtils.ts +++ b/src/cellUtils.ts @@ -121,8 +121,7 @@ export const findLinePos = (cell: ICellModel): number => { }; /** - * Find the line number which contains the PYFLYBY_END_MSG. - * Returns -1 if the PYFLYBY_END_MSG doesn't exist. + * This code extracts non-imports lines from the pyflyby cell. * * @param cell - a cell model */