From 42cdd2e4d7bcec4b81a18e6e556216d640e2d9e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Wed, 11 Oct 2023 11:42:47 +0200 Subject: [PATCH] Make collapseIdentical margin configurable Improve base interfaces --- nbdime/args.py | 8 +++++ nbdime/tests/test_server_extension.py | 1 + nbdime/webapp/nbdimeserver.py | 1 + package.json | 2 +- packages/nbdime/src/common/basepanel.ts | 32 +++++++++++--------- packages/nbdime/src/common/interfaces.ts | 32 +++++++++++++++++--- packages/nbdime/src/common/mergeview.ts | 30 +++++++++++------- packages/nbdime/src/diff/widget/cell.ts | 6 ++++ packages/nbdime/src/diff/widget/metadata.ts | 2 +- packages/nbdime/src/diff/widget/notebook.ts | 3 ++ packages/nbdime/src/diff/widget/output.ts | 2 ++ packages/nbdime/src/merge/widget/cell.ts | 4 +-- packages/nbdime/src/merge/widget/metadata.ts | 4 +-- packages/nbdime/src/merge/widget/notebook.ts | 4 +-- packages/webapp/src/app/diff.ts | 2 ++ packages/webapp/src/app/merge.ts | 2 ++ 16 files changed, 96 insertions(+), 39 deletions(-) diff --git a/nbdime/args.py b/nbdime/args.py index 93108668..6c5b17ef 100644 --- a/nbdime/args.py +++ b/nbdime/args.py @@ -288,6 +288,13 @@ def add_web_args(parser, default_port=8888): default=True, help="show unchanged cells by default" ) + parser.add_argument( + '--identical-lines-margin', + dest='identical_lines_margin', + default=2, + type=int, + help="Margin for collapsing identical lines in editor; set to -1 to deactivate.", + ) def add_diff_args(parser): @@ -539,6 +546,7 @@ def args_for_server(arguments): base_url='base_url', hide_unchanged='hide_unchanged', show_base='show_base', + identical_lines_margin='identical_lines_margin', ) ret = {kmap[k]: v for k, v in vars(arguments).items() if k in kmap} if 'persist' in arguments: diff --git a/nbdime/tests/test_server_extension.py b/nbdime/tests/test_server_extension.py index 664a4f06..5dc0b96d 100644 --- a/nbdime/tests/test_server_extension.py +++ b/nbdime/tests/test_server_extension.py @@ -77,6 +77,7 @@ def test_git_difftool(git_repo2, server_extension_app): "base": "git:", "baseUrl": "/nbdime", "closable": False, + "collapseIdentical": 2, "remote": "", "savable": False, "hideUnchanged": True, diff --git a/nbdime/webapp/nbdimeserver.py b/nbdime/webapp/nbdimeserver.py index f6c157c7..e4d98b6f 100644 --- a/nbdime/webapp/nbdimeserver.py +++ b/nbdime/webapp/nbdimeserver.py @@ -50,6 +50,7 @@ def base_args(self): 'savable': fn is not None, 'baseUrl': self.nbdime_base_url, 'hideUnchanged': self.params.get('hide_unchanged', True), + 'collapseIdentical': self.params.get('identical_lines_margin', 2), } if fn: # For reference, e.g. if user wants to download file diff --git a/package.json b/package.json index 4a806a16..e7c786af 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "updated": "lerna updated", "watch:webapp": "run-p watch:lib watch:app", "watch:app": "lerna exec --stream --scope \"nbdime-webapp\" npm run watch", - "watch:lib": "lerna exec --stream --scope \"nbdime\" --scope \"jupyterlab-rise\" npm run watch" + "watch:lib": "lerna exec --stream --scope \"nbdime\" --scope \"nbdime-jupyterlab\" npm run watch" }, "devDependencies": { "@jupyterlab/buildutils": "4.0.0", diff --git a/packages/nbdime/src/common/basepanel.ts b/packages/nbdime/src/common/basepanel.ts index 17a8566f..0fe6a996 100644 --- a/packages/nbdime/src/common/basepanel.ts +++ b/packages/nbdime/src/common/basepanel.ts @@ -2,35 +2,37 @@ // Distributed under the terms of the Modified BSD License. import { Panel } from '@lumino/widgets'; -import type { IDiffWidgetOptions, IMergeWidgetOptions } from './interfaces'; +import type { + IDiffViewOptions, + IDiffWidgetOptions, + IMergeViewOptions, +} from './interfaces'; import type { CodeEditor } from '@jupyterlab/codeeditor'; /** * Common panel for diff views */ -export class DiffPanel extends Panel { - constructor({ model, editorFactory }: IDiffWidgetOptions) { +export class DiffPanel< + T, + U extends IDiffViewOptions = IDiffViewOptions, +> extends Panel { + constructor({ + model, + editorFactory, + ...viewOptions + }: IDiffWidgetOptions & U) { super(); this._editorFactory = editorFactory; this._model = model; + this._viewOptions = viewOptions as U; } protected _editorFactory: CodeEditor.Factory | undefined; protected _model: T; + protected _viewOptions: U; } /** * Common panel for merge views */ -export class MergePanel extends DiffPanel { - constructor({ - model, - editorFactory, - ...viewOptions - }: IDiffWidgetOptions & IMergeWidgetOptions) { - super({ model, editorFactory }); - this._viewOptions = viewOptions; - } - - protected _viewOptions: IMergeWidgetOptions; -} +export class MergePanel extends DiffPanel {} diff --git a/packages/nbdime/src/common/interfaces.ts b/packages/nbdime/src/common/interfaces.ts index 7053bcd3..f1d76cb3 100644 --- a/packages/nbdime/src/common/interfaces.ts +++ b/packages/nbdime/src/common/interfaces.ts @@ -1,10 +1,22 @@ import type { CodeEditor } from '@jupyterlab/codeeditor'; import type { IRenderMimeRegistry } from '@jupyterlab/rendermime'; +/** + * Diff view options + */ +export interface IDiffViewOptions { + /** + * When true stretches of unchanged text will be collapsed in the text editors. + * When a number is given, this indicates the amount of lines to leave visible + * around such stretches (which defaults to 2). Defaults to true. + */ + collapseIdentical?: boolean | number; +} + /** * Common merge view options */ -export interface IMergeWidgetOptions { +export interface IMergeViewOptions extends IDiffViewOptions { /** * Whether to show the base version (4-panels) or not (3-panels). */ @@ -16,18 +28,28 @@ export interface IMergeWidgetOptions { */ // TODO `T` should be scoped down but more API rework will be needed on the model to achieve that // there is definitely room to rationalize the code with more abstract or mixin classes. -export interface IDiffWidgetOptions { +export interface IDiffWidgetOptions extends IDiffViewOptions { + /** + * Diff model + */ model: T; + /** + * Text editor factory + */ editorFactory?: CodeEditor.Factory; } -export interface IMimeDiffWidgetOptions { - model: T; +export interface IMimeDiffWidgetOptions extends IDiffWidgetOptions { + /** + * Rendermime registry + */ rendermime: IRenderMimeRegistry; - editorFactory?: CodeEditor.Factory; } export interface ICellDiffWidgetOptions extends IMimeDiffWidgetOptions { + /** + * Cell mime type + */ // TODO this seems redundant as mimetype is part of the model mimetype: string; } diff --git a/packages/nbdime/src/common/mergeview.ts b/packages/nbdime/src/common/mergeview.ts index 076be78a..9790b95d 100644 --- a/packages/nbdime/src/common/mergeview.ts +++ b/packages/nbdime/src/common/mergeview.ts @@ -44,7 +44,7 @@ import { createEditorFactory, } from './editor'; -import type { IMergeWidgetOptions } from './interfaces'; +import type { IMergeViewOptions } from './interfaces'; import { valueIn, hasEntries, splitLines } from './util'; @@ -135,7 +135,7 @@ const baseTheme = EditorView.baseTheme({ backgroundColor: 'var(--jp-layout-color2)', border: 'var(--jp-border-width) solid var(--jp-border-color1)', fontSize: '90%', - padding:'0 3px', + padding: '0 3px', borderRadius: '4px', }, }); @@ -515,7 +515,7 @@ const CollapsedRangesField = StateField.define({ /** * Merge view factory options */ -export interface IMergeViewOptions extends Partial { +export interface IMergeViewFactoryOptions extends Partial { /** * Diff between the reference and a remote version */ @@ -538,14 +538,25 @@ export interface IMergeViewOptions extends Partial { /** * A wrapper view for showing StringDiffModels in a MergeView */ -export function createNbdimeMergeView(options: IMergeViewOptions): MergeView { - const { remote, local, merged, readOnly, factory, showBase } = options; +export function createNbdimeMergeView( + options: IMergeViewFactoryOptions, +): MergeView { + const { + remote, + local, + merged, + readOnly, + factory, + collapseIdentical, + showBase, + } = options; let opts: IMergeViewEditorConfiguration = { remote, local, merged, config: { readOnly }, factory: factory ?? createEditorFactory(), + collapseIdentical, showBase, }; @@ -1425,8 +1436,8 @@ export class MergeView extends Panel { const additionalExtensions = inMergeView ? [listener, mergeControlGutter, getCommonEditorExtensions(inMergeView)] : getCommonEditorExtensions(inMergeView); - if(this._collapseIdentical >= 0) { - additionalExtensions.push(CollapsedRangesField) + if (this._collapseIdentical >= 0) { + additionalExtensions.push(CollapsedRangesField); } this._base = new EditorWidget({ @@ -1608,7 +1619,6 @@ export class MergeView extends Panel { * Align the matching lines of the different editors */ alignViews() { - console.log(this._showBase, this._diffViews.length); let lineHeight = this._showBase ? this._base.cm.defaultLineHeight : this._diffViews[0].remoteEditorWidget.cm.defaultLineHeight; @@ -1650,8 +1660,6 @@ export class MergeView extends Panel { if (!this._showBase && i === 0) { return; } - - let side = -1; let line = alignment[i]; if (delta > 0 && line < nLines[i]) { @@ -1668,7 +1676,7 @@ export class MergeView extends Panel { Decoration.widget({ widget: new PaddingWidget(delta * lineHeight), block: true, - side, + side: -1, }), ); } diff --git a/packages/nbdime/src/diff/widget/cell.ts b/packages/nbdime/src/diff/widget/cell.ts index bf33ad34..3cfef585 100644 --- a/packages/nbdime/src/diff/widget/cell.ts +++ b/packages/nbdime/src/diff/widget/cell.ts @@ -105,6 +105,7 @@ export class CellDiffWidget extends DiffPanel { editorClasses: CURR_DIFF_CLASSES, rendermime: this._rendermime, editorFactory: this._editorFactory, + ...this._viewOptions, }); sourceView.addClass(SOURCE_ROW_CLASS); if (model.executionCount) { @@ -122,6 +123,7 @@ export class CellDiffWidget extends DiffPanel { editorClasses: CURR_DIFF_CLASSES, rendermime: this._rendermime, editorFactory: this._editorFactory, + ...this._viewOptions, }); metadataView.addClass(METADATA_ROW_CLASS); this.addWidget(metadataView); @@ -140,6 +142,7 @@ export class CellDiffWidget extends DiffPanel { editorClasses: CURR_DIFF_CLASSES, rendermime: this._rendermime, editorFactory: this._editorFactory, + ...this._viewOptions, }); container.addWidget(outputsWidget); changed = changed || !o.unchanged || o.added || o.deleted; @@ -159,6 +162,7 @@ export class CellDiffWidget extends DiffPanel { editorClasses: CURR_DIFF_CLASSES, rendermime: this._rendermime, editorFactory: this._editorFactory, + ...this._viewOptions, }); target.addWidget(outputsWidget); changed = changed || !o.unchanged || o.added || o.deleted; @@ -217,6 +221,7 @@ export class CellDiffWidget extends DiffPanel { editorClasses, rendermime, editorFactory, + ...viewOptions }: ICellDiffViewOptions): Panel { let view: Panel; if (model instanceof StringDiffModel) { @@ -236,6 +241,7 @@ export class CellDiffWidget extends DiffPanel { inner = createNbdimeMergeView({ remote: model, factory: editorFactory, + ...viewOptions, }); } if (model.collapsible) { diff --git a/packages/nbdime/src/diff/widget/metadata.ts b/packages/nbdime/src/diff/widget/metadata.ts index 57e7874d..cfacf5fe 100644 --- a/packages/nbdime/src/diff/widget/metadata.ts +++ b/packages/nbdime/src/diff/widget/metadata.ts @@ -22,7 +22,6 @@ const ROOT_METADATA_CLASS = 'jp-Metadata-diff'; * MetadataWidget for changes to Notebook-level metadata */ export class MetadataDiffWidget extends DiffPanel { - // TODO improve typing hierarchy to avoid `Omit` constructor(options: IDiffWidgetOptions) { super(options); console.assert(!this._model.added && !this._model.deleted); @@ -37,6 +36,7 @@ export class MetadataDiffWidget extends DiffPanel { let view: Widget = createNbdimeMergeView({ remote: model, factory: this._editorFactory, + ...this._viewOptions, }); if (model.collapsible) { view = new CollapsiblePanel( diff --git a/packages/nbdime/src/diff/widget/notebook.ts b/packages/nbdime/src/diff/widget/notebook.ts index c3f1d328..42098351 100644 --- a/packages/nbdime/src/diff/widget/notebook.ts +++ b/packages/nbdime/src/diff/widget/notebook.ts @@ -53,6 +53,7 @@ export class NotebookDiffWidget extends DiffPanel { new MetadataDiffWidget({ model: model.metadata, editorFactory: this._editorFactory, + ...this._viewOptions, }), ); } @@ -67,6 +68,7 @@ export class NotebookDiffWidget extends DiffPanel { rendermime, mimetype: model.mimetype, editorFactory: this._editorFactory, + ...this._viewOptions, }), ); } else { @@ -84,6 +86,7 @@ export class NotebookDiffWidget extends DiffPanel { rendermime, mimetype: model.mimetype, editorFactory: this._editorFactory, + ...this._viewOptions, }), ); } diff --git a/packages/nbdime/src/diff/widget/output.ts b/packages/nbdime/src/diff/widget/output.ts index 4cc52090..aed8fb90 100644 --- a/packages/nbdime/src/diff/widget/output.ts +++ b/packages/nbdime/src/diff/widget/output.ts @@ -233,6 +233,7 @@ export class OutputPanel extends DiffPanel { view = createNbdimeMergeView({ remote: stringModel, factory: this._editorFactory, + ...this._viewOptions, }); } } @@ -241,6 +242,7 @@ export class OutputPanel extends DiffPanel { view = createNbdimeMergeView({ remote: model.stringify(), factory: this._editorFactory, + ...this._viewOptions, }); } return view; diff --git a/packages/nbdime/src/merge/widget/cell.ts b/packages/nbdime/src/merge/widget/cell.ts index 45a9c5f2..5eeb4f42 100644 --- a/packages/nbdime/src/merge/widget/cell.ts +++ b/packages/nbdime/src/merge/widget/cell.ts @@ -18,7 +18,7 @@ import { DragPanel } from '../../common/dragpanel'; import type { ICellDiffWidgetOptions, - IMergeWidgetOptions, + IMergeViewOptions, } from '../../common/interfaces'; import { createNbdimeMergeView, MergeView } from '../../common/mergeview'; @@ -87,7 +87,7 @@ export class CellMergeWidget extends MergePanel { merged, readOnly, ...others - }: ICellMergeViewOptions & IMergeWidgetOptions): Widget | null { + }: ICellMergeViewOptions & IMergeViewOptions): Widget | null { let view: Widget | null = null; if (merged instanceof StringDiffModel) { view = createNbdimeMergeView({ diff --git a/packages/nbdime/src/merge/widget/metadata.ts b/packages/nbdime/src/merge/widget/metadata.ts index 8f857d4f..6503c8d6 100644 --- a/packages/nbdime/src/merge/widget/metadata.ts +++ b/packages/nbdime/src/merge/widget/metadata.ts @@ -6,7 +6,7 @@ import type * as nbformat from '@jupyterlab/nbformat'; import type { IDiffWidgetOptions, - IMergeWidgetOptions, + IMergeViewOptions, } from '../../common/interfaces'; import { createNbdimeMergeView, MergeView } from '../../common/mergeview'; @@ -24,7 +24,7 @@ const ROOT_METADATA_CLASS = 'jp-Metadata-diff'; */ export class MetadataMergeWidget extends MergePanel { constructor( - options: IDiffWidgetOptions & IMergeWidgetOptions, + options: IDiffWidgetOptions & IMergeViewOptions, ) { super(options); this.addClass(ROOT_METADATA_CLASS); diff --git a/packages/nbdime/src/merge/widget/notebook.ts b/packages/nbdime/src/merge/widget/notebook.ts index f8d5a789..586c7cbe 100644 --- a/packages/nbdime/src/merge/widget/notebook.ts +++ b/packages/nbdime/src/merge/widget/notebook.ts @@ -9,7 +9,7 @@ import type { IRenderMimeRegistry } from '@jupyterlab/rendermime'; import { MergePanel } from '../../common/basepanel'; import type { - IMergeWidgetOptions, + IMergeViewOptions, IMimeDiffWidgetOptions, } from '../../common/interfaces'; @@ -37,7 +37,7 @@ export class NotebookMergeWidget extends MergePanel { constructor({ rendermime, ...options - }: IMimeDiffWidgetOptions & IMergeWidgetOptions) { + }: IMimeDiffWidgetOptions & IMergeViewOptions) { super(options); this._rendermime = rendermime; diff --git a/packages/webapp/src/app/diff.ts b/packages/webapp/src/app/diff.ts index 00221d0d..773f6a43 100644 --- a/packages/webapp/src/app/diff.ts +++ b/packages/webapp/src/app/diff.ts @@ -68,12 +68,14 @@ function showDiff(data: { sanitizer: new Sanitizer(), latexTypesetter: new MathJaxTypesetter(), }); + const collapseIdentical = getConfigOption('collapseIdentical', 2); let nbdModel = new NotebookDiffModel(data.base, data.diff); let nbdWidget = new NotebookDiffWidget({ model: nbdModel, rendermime, editorFactory: createEditorFactory(), + collapseIdentical, }); let root = document.getElementById('nbdime-root'); diff --git a/packages/webapp/src/app/merge.ts b/packages/webapp/src/app/merge.ts index 727d268c..4e308558 100644 --- a/packages/webapp/src/app/merge.ts +++ b/packages/webapp/src/app/merge.ts @@ -54,6 +54,7 @@ function showMerge(data: { sanitizer: new Sanitizer(), }); + const collapseIdentical = getConfigOption('collapseIdentical', 2); const showBase = getConfigOption('showBase', true); if (!showBase) { @@ -65,6 +66,7 @@ function showMerge(data: { model: nbmModel, rendermime, editorFactory: createEditorFactory(), + collapseIdentical, showBase, });