Skip to content

Commit

Permalink
Make collapseIdentical margin configurable (#707)
Browse files Browse the repository at this point in the history
Improve base interfaces
  • Loading branch information
fcollonval committed Oct 12, 2023
1 parent 228b6b0 commit b306a86
Show file tree
Hide file tree
Showing 16 changed files with 96 additions and 39 deletions.
8 changes: 8 additions & 0 deletions nbdime/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions nbdime/tests/test_server_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions nbdime/webapp/nbdimeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
32 changes: 17 additions & 15 deletions packages/nbdime/src/common/basepanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> extends Panel {
constructor({ model, editorFactory }: IDiffWidgetOptions<T>) {
export class DiffPanel<
T,
U extends IDiffViewOptions = IDiffViewOptions,
> extends Panel {
constructor({
model,
editorFactory,
...viewOptions
}: IDiffWidgetOptions<T> & 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<T> extends DiffPanel<T> {
constructor({
model,
editorFactory,
...viewOptions
}: IDiffWidgetOptions<T> & IMergeWidgetOptions) {
super({ model, editorFactory });
this._viewOptions = viewOptions;
}

protected _viewOptions: IMergeWidgetOptions;
}
export class MergePanel<T> extends DiffPanel<T, IMergeViewOptions> {}
32 changes: 27 additions & 5 deletions packages/nbdime/src/common/interfaces.ts
Original file line number Diff line number Diff line change
@@ -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).
*/
Expand All @@ -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<T> {
export interface IDiffWidgetOptions<T> extends IDiffViewOptions {
/**
* Diff model
*/
model: T;
/**
* Text editor factory
*/
editorFactory?: CodeEditor.Factory;
}

export interface IMimeDiffWidgetOptions<T> {
model: T;
export interface IMimeDiffWidgetOptions<T> extends IDiffWidgetOptions<T> {
/**
* Rendermime registry
*/
rendermime: IRenderMimeRegistry;
editorFactory?: CodeEditor.Factory;
}

export interface ICellDiffWidgetOptions<T> extends IMimeDiffWidgetOptions<T> {
/**
* Cell mime type
*/
// TODO this seems redundant as mimetype is part of the model
mimetype: string;
}
30 changes: 19 additions & 11 deletions packages/nbdime/src/common/mergeview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
createEditorFactory,
} from './editor';

import type { IMergeWidgetOptions } from './interfaces';
import type { IMergeViewOptions } from './interfaces';

import { valueIn, hasEntries, splitLines } from './util';

Expand Down Expand Up @@ -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',
},
});
Expand Down Expand Up @@ -515,7 +515,7 @@ const CollapsedRangesField = StateField.define<DecorationSet>({
/**
* Merge view factory options
*/
export interface IMergeViewOptions extends Partial<IMergeWidgetOptions> {
export interface IMergeViewFactoryOptions extends Partial<IMergeViewOptions> {
/**
* Diff between the reference and a remote version
*/
Expand All @@ -538,14 +538,25 @@ export interface IMergeViewOptions extends Partial<IMergeWidgetOptions> {
/**
* 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,
};

Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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]) {
Expand All @@ -1668,7 +1676,7 @@ export class MergeView extends Panel {
Decoration.widget({
widget: new PaddingWidget(delta * lineHeight),
block: true,
side,
side: -1,
}),
);
}
Expand Down
6 changes: 6 additions & 0 deletions packages/nbdime/src/diff/widget/cell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export class CellDiffWidget extends DiffPanel<CellDiffModel> {
editorClasses: CURR_DIFF_CLASSES,
rendermime: this._rendermime,
editorFactory: this._editorFactory,
...this._viewOptions,
});
sourceView.addClass(SOURCE_ROW_CLASS);
if (model.executionCount) {
Expand All @@ -122,6 +123,7 @@ export class CellDiffWidget extends DiffPanel<CellDiffModel> {
editorClasses: CURR_DIFF_CLASSES,
rendermime: this._rendermime,
editorFactory: this._editorFactory,
...this._viewOptions,
});
metadataView.addClass(METADATA_ROW_CLASS);
this.addWidget(metadataView);
Expand All @@ -140,6 +142,7 @@ export class CellDiffWidget extends DiffPanel<CellDiffModel> {
editorClasses: CURR_DIFF_CLASSES,
rendermime: this._rendermime,
editorFactory: this._editorFactory,
...this._viewOptions,
});
container.addWidget(outputsWidget);
changed = changed || !o.unchanged || o.added || o.deleted;
Expand All @@ -159,6 +162,7 @@ export class CellDiffWidget extends DiffPanel<CellDiffModel> {
editorClasses: CURR_DIFF_CLASSES,
rendermime: this._rendermime,
editorFactory: this._editorFactory,
...this._viewOptions,
});
target.addWidget(outputsWidget);
changed = changed || !o.unchanged || o.added || o.deleted;
Expand Down Expand Up @@ -217,6 +221,7 @@ export class CellDiffWidget extends DiffPanel<CellDiffModel> {
editorClasses,
rendermime,
editorFactory,
...viewOptions
}: ICellDiffViewOptions): Panel {
let view: Panel;
if (model instanceof StringDiffModel) {
Expand All @@ -236,6 +241,7 @@ export class CellDiffWidget extends DiffPanel<CellDiffModel> {
inner = createNbdimeMergeView({
remote: model,
factory: editorFactory,
...viewOptions,
});
}
if (model.collapsible) {
Expand Down
2 changes: 1 addition & 1 deletion packages/nbdime/src/diff/widget/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const ROOT_METADATA_CLASS = 'jp-Metadata-diff';
* MetadataWidget for changes to Notebook-level metadata
*/
export class MetadataDiffWidget extends DiffPanel<IStringDiffModel> {
// TODO improve typing hierarchy to avoid `Omit`
constructor(options: IDiffWidgetOptions<IStringDiffModel>) {
super(options);
console.assert(!this._model.added && !this._model.deleted);
Expand All @@ -37,6 +36,7 @@ export class MetadataDiffWidget extends DiffPanel<IStringDiffModel> {
let view: Widget = createNbdimeMergeView({
remote: model,
factory: this._editorFactory,
...this._viewOptions,
});
if (model.collapsible) {
view = new CollapsiblePanel(
Expand Down
3 changes: 3 additions & 0 deletions packages/nbdime/src/diff/widget/notebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
new MetadataDiffWidget({
model: model.metadata,
editorFactory: this._editorFactory,
...this._viewOptions,
}),
);
}
Expand All @@ -67,6 +68,7 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
rendermime,
mimetype: model.mimetype,
editorFactory: this._editorFactory,
...this._viewOptions,
}),
);
} else {
Expand All @@ -84,6 +86,7 @@ export class NotebookDiffWidget extends DiffPanel<NotebookDiffModel> {
rendermime,
mimetype: model.mimetype,
editorFactory: this._editorFactory,
...this._viewOptions,
}),
);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/nbdime/src/diff/widget/output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ export class OutputPanel extends DiffPanel<OutputDiffModel> {
view = createNbdimeMergeView({
remote: stringModel,
factory: this._editorFactory,
...this._viewOptions,
});
}
}
Expand All @@ -241,6 +242,7 @@ export class OutputPanel extends DiffPanel<OutputDiffModel> {
view = createNbdimeMergeView({
remote: model.stringify(),
factory: this._editorFactory,
...this._viewOptions,
});
}
return view;
Expand Down
4 changes: 2 additions & 2 deletions packages/nbdime/src/merge/widget/cell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { DragPanel } from '../../common/dragpanel';

import type {
ICellDiffWidgetOptions,
IMergeWidgetOptions,
IMergeViewOptions,
} from '../../common/interfaces';

import { createNbdimeMergeView, MergeView } from '../../common/mergeview';
Expand Down Expand Up @@ -87,7 +87,7 @@ export class CellMergeWidget extends MergePanel<CellMergeModel> {
merged,
readOnly,
...others
}: ICellMergeViewOptions & IMergeWidgetOptions): Widget | null {
}: ICellMergeViewOptions & IMergeViewOptions): Widget | null {
let view: Widget | null = null;
if (merged instanceof StringDiffModel) {
view = createNbdimeMergeView({
Expand Down
4 changes: 2 additions & 2 deletions packages/nbdime/src/merge/widget/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -24,7 +24,7 @@ const ROOT_METADATA_CLASS = 'jp-Metadata-diff';
*/
export class MetadataMergeWidget extends MergePanel<MetadataMergeModel> {
constructor(
options: IDiffWidgetOptions<MetadataMergeModel> & IMergeWidgetOptions,
options: IDiffWidgetOptions<MetadataMergeModel> & IMergeViewOptions,
) {
super(options);
this.addClass(ROOT_METADATA_CLASS);
Expand Down

0 comments on commit b306a86

Please sign in to comment.