From 590c313749a9377a561b47a0eafd88e3fe50724b Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Tue, 25 Nov 2025 16:45:35 +0100 Subject: [PATCH 1/8] clean outputs before commit option --- jupyterlab_git/git.py | 50 +++++++++++++++++++++++++++++++++++++ jupyterlab_git/handlers.py | 40 +++++++++++++++++++++++++++++ src/components/GitPanel.tsx | 38 ++++++++++++++++++++++++++++ src/model.ts | 41 ++++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index a4ae5e9e8..390c2f6bd 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -1182,6 +1182,56 @@ async def merge(self, branch: str, path: str) -> dict: return {"code": code, "command": " ".join(cmd), "message": error} return {"code": code, "message": output.strip()} + async def check_notebooks_with_outputs(self, path): + import nbformat, os + + code, stdout, _ = await self.__execute( + ["git", "diff", "--cached", "--name-only", "--diff-filter=ACM"], cwd=path + ) + staged_files = stdout.splitlines() + notebooks = [f for f in staged_files if f.endswith(".ipynb")] + + dirty_notebooks = [] + + for nb_path in notebooks: + full_nb_path = os.path.join(path, nb_path) + try: + nb = nbformat.read(full_nb_path, as_version=nbformat.NO_CONVERT) + for cell in nb.get("cells", []): + if cell.get("cell_type") == "code" and cell.get("outputs"): + dirty_notebooks.append(nb_path) + break + except Exception: + pass + + return { + "notebooks_with_outputs": dirty_notebooks, + "has_outputs": len(dirty_notebooks) > 0, + } + + async def strip_notebook_outputs(self, notebooks: list, repo_path: str): + for nb_path in notebooks: + full_path = os.path.join(repo_path, nb_path) + + try: + # Clear outputs using nbconvert + subprocess.run( + [ + "jupyter", + "nbconvert", + "--ClearOutputPreprocessor.enabled=True", + "--inplace", + full_path, + ], + check=True, + ) + + # Re-stage the cleaned notebook + subprocess.run(["git", "-C", repo_path, "add", full_path], check=True) + + except Exception as e: + print(f"Failed: {nb_path}: {e}") + async def commit(self, commit_msg, amend, path, author=None): """ Execute git commit command & return the result. diff --git a/jupyterlab_git/handlers.py b/jupyterlab_git/handlers.py index c6b270ac1..d463bf97d 100644 --- a/jupyterlab_git/handlers.py +++ b/jupyterlab_git/handlers.py @@ -599,6 +599,44 @@ async def post(self, path: str = ""): self.finish(json.dumps(body)) +class GitStripNotebooksHandler(GitHandler): + """Handler to strip outputs from notebooks in a repository.""" + + @tornado.web.authenticated + async def post(self, path: str = ""): + """ + POST request handler to strip outputs from notebooks. + """ + data = self.get_json_body() + notebooks = data.get("notebooks", []) + + try: + await self.git.strip_notebook_outputs(notebooks, self.url2localpath(path)) + self.set_status(200) + self.finish(json.dumps({"code": 0, "message": "Notebooks stripped"})) + except Exception as e: + self.set_status(500) + self.finish( + json.dumps( + { + "code": 1, + "message": f"Failed to strip notebook outputs: {str(e)}", + } + ) + ) + + +class GitCheckNotebooksHandler(GitHandler): + """ + Handler to check staged notebooks for outputs. + """ + + @tornado.web.authenticated + async def get(self, path: str = ""): + body = await self.git.check_notebooks_with_outputs(self.url2localpath(path)) + self.finish(json.dumps(body)) + + class GitUpstreamHandler(GitHandler): @tornado.web.authenticated async def post(self, path: str = ""): @@ -1182,6 +1220,8 @@ def setup_handlers(web_app): ("/stash_pop", GitStashPopHandler), ("/stash_apply", GitStashApplyHandler), ("/submodules", GitSubmodulesHandler), + ("/check_notebooks", GitCheckNotebooksHandler), + ("/strip_notebooks", GitStripNotebooksHandler), ] handlers = [ diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index 61cd1e8b7..d05fbc4c4 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -803,9 +803,47 @@ export class GitPanel extends React.Component { ): Promise => { const errorMsg = this.props.trans.__('Failed to commit changes.'); let id: string | null = notificationId ?? null; + try { const author = await this._hasIdentity(this.props.model.pathRepository); + const notebooksWithOutputs = + await this.props.model.checkNotebooksForOutputs(); + + if (notebooksWithOutputs.length > 0) { + const dialog = new Dialog({ + title: this.props.trans.__('Notebook outputs detected'), + body: `You are about to commit ${notebooksWithOutputs.length} notebook(s) with outputs. + Would you like to clean them before committing?`, + buttons: [ + Dialog.cancelButton({ + label: this.props.trans.__('Commit Anyway') + }), + Dialog.okButton({ + label: this.props.trans.__('Clean & Commit') + }) + ] + }); + + const result = await dialog.launch(); + dialog.dispose(); + + if (result.button.label === this.props.trans.__('Commit Anyway')) { + } else if ( + result.button.label === this.props.trans.__('Clean & Commit') + ) { + id = Notification.emit( + this.props.trans.__('Cleaning notebook outputs…'), + 'in-progress', + { autoClose: false } + ); + + await this.props.model.stripNotebooksOutputs(notebooksWithOutputs); + } else { + return; + } + } + const notificationMsg = this.props.trans.__('Committing changes...'); if (id !== null) { Notification.update({ diff --git a/src/model.ts b/src/model.ts index 114fa0f39..02c0a301b 100644 --- a/src/model.ts +++ b/src/model.ts @@ -733,6 +733,47 @@ export class GitExtension implements IGitExtension { await this.refresh(); } + /** + * Check staged notebooks for outputs. + * + * @returns A promise resolving to an array of notebook paths that have outputs + * + * @throws {Git.NotInRepository} If the current path is not a Git repository + * @throws {Git.GitResponseError} If the server response is not ok + * @throws {ServerConnection.NetworkError} If the request cannot be made + */ + async checkNotebooksForOutputs(): Promise { + const path = await this._getPathRepository(); + + return this._taskHandler.execute('git:check-notebooks', async () => { + const result = await requestAPI<{ notebooks_with_outputs: string[] }>( + URLExt.join(path, 'check_notebooks') + ); + return result.notebooks_with_outputs; + }); + } + + /** + * Strip outputs from the given staged notebooks. + * + * @param notebooks - Array of notebook paths to clean + * + * @returns A promise resolving when the operation completes + * + * @throws {Git.NotInRepository} If the current path is not a Git repository + * @throws {Git.GitResponseError} If the server response is not ok + * @throws {ServerConnection.NetworkError} If the request cannot be made + */ + async stripNotebooksOutputs(notebooks: string[]): Promise { + const path = await this._getPathRepository(); + + await this._taskHandler.execute('git:strip-notebooks', async () => { + await requestAPI(URLExt.join(path, 'strip_notebooks'), 'POST', { + notebooks + }); + }); + } + /** * Get (or set) Git configuration options. * From 8dd74dc1244084581497c009a6f29783a8c3cc42 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Wed, 26 Nov 2025 16:42:45 +0100 Subject: [PATCH 2/8] add setting to always clear outputs before commits --- schema/plugin.json | 6 ++++++ src/components/GitPanel.tsx | 12 +++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/schema/plugin.json b/schema/plugin.json index c0dba9365..c3cc76e1f 100644 --- a/schema/plugin.json +++ b/schema/plugin.json @@ -82,6 +82,12 @@ "title": "Hide hidden file warning", "description": "If true, the warning popup when opening the .gitignore file without hidden files will not be displayed.", "default": false + }, + "clearOutputsBeforeCommit":{ + "type": "boolean", + "title": "Clear outputs before commit", + "description": "If true, notebook outputs will be cleared before committing.", + "default": false } }, "jupyter.lab.shortcuts": [ diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index d05fbc4c4..9cc737e2b 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -810,7 +810,10 @@ export class GitPanel extends React.Component { const notebooksWithOutputs = await this.props.model.checkNotebooksForOutputs(); - if (notebooksWithOutputs.length > 0) { + if ( + notebooksWithOutputs.length > 0 && + !this.props.settings.composite['clearOutputsBeforeCommit'] + ) { const dialog = new Dialog({ title: this.props.trans.__('Notebook outputs detected'), body: `You are about to commit ${notebooksWithOutputs.length} notebook(s) with outputs. @@ -842,6 +845,13 @@ export class GitPanel extends React.Component { } else { return; } + } else if (this.props.settings.composite['clearOutputsBeforeCommit']) { + id = Notification.emit( + this.props.trans.__('Cleaning notebook outputs…'), + 'in-progress', + { autoClose: false } + ); + await this.props.model.stripNotebooksOutputs(notebooksWithOutputs); } const notificationMsg = this.props.trans.__('Committing changes...'); From 61dc4bd915aa95d4479ab36b9e7d2aef36ba7387 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Thu, 27 Nov 2025 09:57:03 +0100 Subject: [PATCH 3/8] lint --- schema/plugin.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schema/plugin.json b/schema/plugin.json index c3cc76e1f..2df359161 100644 --- a/schema/plugin.json +++ b/schema/plugin.json @@ -83,7 +83,7 @@ "description": "If true, the warning popup when opening the .gitignore file without hidden files will not be displayed.", "default": false }, - "clearOutputsBeforeCommit":{ + "clearOutputsBeforeCommit": { "type": "boolean", "title": "Clear outputs before commit", "description": "If true, notebook outputs will be cleared before committing.", From 989f3c8716e97aa18a43f97fee6918e4e7bc79ba Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Thu, 27 Nov 2025 15:05:12 +0100 Subject: [PATCH 4/8] remove empty if block --- src/components/GitPanel.tsx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index 9cc737e2b..593ddb04b 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -817,7 +817,7 @@ export class GitPanel extends React.Component { const dialog = new Dialog({ title: this.props.trans.__('Notebook outputs detected'), body: `You are about to commit ${notebooksWithOutputs.length} notebook(s) with outputs. - Would you like to clean them before committing?`, + Would you like to clean them before committing? If you prefer not to be asked every time, you can enable the "Clear outputs before commit" option in the settings.`, buttons: [ Dialog.cancelButton({ label: this.props.trans.__('Commit Anyway') @@ -831,10 +831,14 @@ export class GitPanel extends React.Component { const result = await dialog.launch(); dialog.dispose(); - if (result.button.label === this.props.trans.__('Commit Anyway')) { - } else if ( - result.button.label === this.props.trans.__('Clean & Commit') - ) { + if (result.button.label === this.props.trans.__('Cancel')) { + return; + } + const accepted = + typeof result.button.accept === 'boolean' + ? result.button.accept + : result.button.label === this.props.trans.__('Clean & Commit'); + if (accepted) { id = Notification.emit( this.props.trans.__('Cleaning notebook outputs…'), 'in-progress', @@ -842,8 +846,6 @@ export class GitPanel extends React.Component { ); await this.props.model.stripNotebooksOutputs(notebooksWithOutputs); - } else { - return; } } else if (this.props.settings.composite['clearOutputsBeforeCommit']) { id = Notification.emit( From 32411ae01e11bd8a467040701579bad11005eedb Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Fri, 28 Nov 2025 11:49:35 +0100 Subject: [PATCH 5/8] requested changes --- jupyterlab_git/__init__.py | 13 +++++++++++ jupyterlab_git/git.py | 18 ++++++--------- schema/plugin.json | 5 +++-- src/components/GitPanel.tsx | 44 ++++++++++++++++++++++++------------- 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/jupyterlab_git/__init__.py b/jupyterlab_git/__init__.py index 7e1762e25..78d7514c1 100644 --- a/jupyterlab_git/__init__.py +++ b/jupyterlab_git/__init__.py @@ -51,6 +51,19 @@ class JupyterLabGit(Configurable): config=True, ) + output_cleaning_command = Unicode( + "jupyter nbconvert", + help="Notebook cleaning command. Configurable by server admin.", + config=True, + ) + + # Extra options to pass to the cleaning tool + output_cleaning_options = Unicode( + "--ClearOutputPreprocessor.enabled=True --inplace", + help="Extra command-line options to pass to the cleaning tool.", + config=True, + ) + @default("credential_helper") def _credential_helper_default(self): return "cache --timeout=3600" diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index 390c2f6bd..8c7d6c546 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -1214,17 +1214,13 @@ async def strip_notebook_outputs(self, notebooks: list, repo_path: str): full_path = os.path.join(repo_path, nb_path) try: - # Clear outputs using nbconvert - subprocess.run( - [ - "jupyter", - "nbconvert", - "--ClearOutputPreprocessor.enabled=True", - "--inplace", - full_path, - ], - check=True, - ) + cmd = shlex.split(self._config.output_cleaning_command) + options = shlex.split(self._config.output_cleaning_options) + + full_cmd = cmd + options + [full_path] + + # Run the cleaning command + subprocess.run(full_cmd, check=True) # Re-stage the cleaned notebook subprocess.run(["git", "-C", repo_path, "add", full_path], check=True) diff --git a/schema/plugin.json b/schema/plugin.json index 2df359161..77f28cf69 100644 --- a/schema/plugin.json +++ b/schema/plugin.json @@ -86,8 +86,9 @@ "clearOutputsBeforeCommit": { "type": "boolean", "title": "Clear outputs before commit", - "description": "If true, notebook outputs will be cleared before committing.", - "default": false + "description": "If true, notebook outputs will be cleared before committing. If false, outputs are kept. If null, ask each time.", + "default": null, + "nullable": true } }, "jupyter.lab.shortcuts": [ diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index 593ddb04b..d167a47d2 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -35,6 +35,7 @@ import { HistorySideBar } from './HistorySideBar'; import { RebaseAction } from './RebaseAction'; import { Toolbar } from './Toolbar'; import { WarningBox } from './WarningBox'; +import { Widget } from '@lumino/widgets'; /** * Interface describing component properties. @@ -810,22 +811,29 @@ export class GitPanel extends React.Component { const notebooksWithOutputs = await this.props.model.checkNotebooksForOutputs(); - if ( - notebooksWithOutputs.length > 0 && - !this.props.settings.composite['clearOutputsBeforeCommit'] - ) { + const clearSetting = + this.props.settings.composite['clearOutputsBeforeCommit']; + if (notebooksWithOutputs.length > 0 && clearSetting == null) { + const bodyWidget = new Widget(); + bodyWidget.node.innerHTML = ` +
+

Clear all outputs before committing?

+ +
+ `; + const dialog = new Dialog({ title: this.props.trans.__('Notebook outputs detected'), - body: `You are about to commit ${notebooksWithOutputs.length} notebook(s) with outputs. - Would you like to clean them before committing? If you prefer not to be asked every time, you can enable the "Clear outputs before commit" option in the settings.`, + body: bodyWidget, buttons: [ Dialog.cancelButton({ - label: this.props.trans.__('Commit Anyway') + label: this.props.trans.__('Keep Outputs & Commit') }), - Dialog.okButton({ - label: this.props.trans.__('Clean & Commit') - }) - ] + Dialog.okButton({ label: this.props.trans.__('Clean & Commit') }) + ], + defaultButton: 0 }); const result = await dialog.launch(); @@ -835,9 +843,14 @@ export class GitPanel extends React.Component { return; } const accepted = - typeof result.button.accept === 'boolean' - ? result.button.accept - : result.button.label === this.props.trans.__('Clean & Commit'); + result.button.label === this.props.trans.__('Clean & Commit'); + const checkbox = + bodyWidget.node.querySelector('#dontAskAgain'); + + // Remember the user’s choice if checkbox is checked + if (checkbox?.checked) { + this.props.settings.set('clearOutputsBeforeCommit', accepted); + } if (accepted) { id = Notification.emit( this.props.trans.__('Cleaning notebook outputs…'), @@ -847,7 +860,8 @@ export class GitPanel extends React.Component { await this.props.model.stripNotebooksOutputs(notebooksWithOutputs); } - } else if (this.props.settings.composite['clearOutputsBeforeCommit']) { + } else if (clearSetting === true) { + // Always clean before commit id = Notification.emit( this.props.trans.__('Cleaning notebook outputs…'), 'in-progress', From ffeb762b47b893534887d756e1cd898ecd1104f6 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Fri, 28 Nov 2025 15:26:29 +0100 Subject: [PATCH 6/8] lint --- src/components/GitPanel.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index d167a47d2..9ee5558d1 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -813,7 +813,10 @@ export class GitPanel extends React.Component { const clearSetting = this.props.settings.composite['clearOutputsBeforeCommit']; - if (notebooksWithOutputs.length > 0 && clearSetting == null) { + if ( + notebooksWithOutputs.length > 0 && + (clearSetting === null || clearSetting === undefined) + ) { const bodyWidget = new Widget(); bodyWidget.node.innerHTML = `
From e5370b7a1fef3e14c33091676c820c32087bfbf2 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Mon, 1 Dec 2025 10:02:31 +0100 Subject: [PATCH 7/8] use dialog's checkbox and fix tests --- .../test-components/GitPanel.spec.tsx | 7 +++++++ src/components/GitPanel.tsx | 20 +++++-------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/__tests__/test-components/GitPanel.spec.tsx b/src/__tests__/test-components/GitPanel.spec.tsx index defda4230..4c0f0b7e9 100644 --- a/src/__tests__/test-components/GitPanel.spec.tsx +++ b/src/__tests__/test-components/GitPanel.spec.tsx @@ -188,6 +188,8 @@ describe('GitPanel', () => { it('should commit when commit message is provided', async () => { configSpy.mockResolvedValue({ options: commitUser }); + props.model.checkNotebooksForOutputs = jest.fn().mockResolvedValue([]); + await userEvent.type(screen.getAllByRole('textbox')[0], commitSummary); await userEvent.type( screen.getAllByRole('textbox')[1], @@ -223,6 +225,7 @@ describe('GitPanel', () => { it('should prompt for user identity if explicitly configured', async () => { configSpy.mockResolvedValue({ options: commitUser }); + props.model.checkNotebooksForOutputs = jest.fn().mockResolvedValue([]); props.settings = MockSettings(false, true) as any; renderResult.rerender(); @@ -245,6 +248,7 @@ describe('GitPanel', () => { it('should prompt for user identity if user.name is not set', async () => { configSpy.mockImplementation(mockConfigImplementation('user.email')); mockUtils.showDialog.mockResolvedValue(dialogValue); + props.model.checkNotebooksForOutputs = jest.fn().mockResolvedValue([]); await userEvent.type(screen.getAllByRole('textbox')[0], commitSummary); await userEvent.click(screen.getByRole('button', { name: 'Commit' })); @@ -261,6 +265,7 @@ describe('GitPanel', () => { it('should prompt for user identity if user.email is not set', async () => { configSpy.mockImplementation(mockConfigImplementation('user.name')); mockUtils.showDialog.mockResolvedValue(dialogValue); + props.model.checkNotebooksForOutputs = jest.fn().mockResolvedValue([]); await userEvent.type(screen.getAllByRole('textbox')[0], commitSummary); await userEvent.click(screen.getByRole('button', { name: 'Commit' })); @@ -280,6 +285,7 @@ describe('GitPanel', () => { configSpy.mockImplementation(mockConfigImplementation('user.email')); mockUtils.showDialog.mockResolvedValue(dialogValue); + props.model.checkNotebooksForOutputs = jest.fn().mockResolvedValue([]); await userEvent.type(screen.getAllByRole('textbox')[0], commitSummary); await userEvent.click(screen.getByRole('button', { name: 'Commit' })); @@ -298,6 +304,7 @@ describe('GitPanel', () => { renderResult.rerender(); configSpy.mockImplementation(mockConfigImplementation('user.name')); mockUtils.showDialog.mockResolvedValue(dialogValue); + props.model.checkNotebooksForOutputs = jest.fn().mockResolvedValue([]); await userEvent.type(screen.getAllByRole('textbox')[0], commitSummary); await userEvent.click(screen.getByRole('button', { name: 'Commit' })); diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index 9ee5558d1..9cc0bfe0e 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -35,7 +35,6 @@ import { HistorySideBar } from './HistorySideBar'; import { RebaseAction } from './RebaseAction'; import { Toolbar } from './Toolbar'; import { WarningBox } from './WarningBox'; -import { Widget } from '@lumino/widgets'; /** * Interface describing component properties. @@ -817,19 +816,12 @@ export class GitPanel extends React.Component { notebooksWithOutputs.length > 0 && (clearSetting === null || clearSetting === undefined) ) { - const bodyWidget = new Widget(); - bodyWidget.node.innerHTML = ` -
-

Clear all outputs before committing?

- -
- `; - const dialog = new Dialog({ title: this.props.trans.__('Notebook outputs detected'), - body: bodyWidget, + checkbox: { + label: this.props.trans.__('Clear all outputs before committing?'), + checked: false + }, buttons: [ Dialog.cancelButton({ label: this.props.trans.__('Keep Outputs & Commit') @@ -847,11 +839,9 @@ export class GitPanel extends React.Component { } const accepted = result.button.label === this.props.trans.__('Clean & Commit'); - const checkbox = - bodyWidget.node.querySelector('#dontAskAgain'); // Remember the user’s choice if checkbox is checked - if (checkbox?.checked) { + if (result?.isChecked) { this.props.settings.set('clearOutputsBeforeCommit', accepted); } if (accepted) { From a3dc3b38c11061f7a49a0f28230cd10c1315b709 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Mon, 1 Dec 2025 10:11:24 +0100 Subject: [PATCH 8/8] chekc links --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2b6aec5a6..4203be293 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -169,4 +169,4 @@ jobs: - uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1 - uses: jupyterlab/maintainer-tools/.github/actions/check-links@v1 with: - ignore_links: "https://www.linkedin.com/.* https://fellowship.mlh.io/.* https://github.com/.*" + ignore_links: "https://www.linkedin.com/.* https://fellowship.mlh.io/.* https://github.com/.* https://amazon.com/.*"