Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make collapseIdentical margin configurable #707

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 type {
ICellDiffWidgetOptions,
IMergeWidgetOptions,
IMergeViewOptions,
} from '../../common/interfaces';

import { createNbdimeMergeView, MergeView } from '../../common/mergeview';
Expand Down Expand Up @@ -87,7 +87,7 @@
merged,
readOnly,
...others
}: ICellMergeViewOptions & IMergeWidgetOptions): Widget | null {
}: ICellMergeViewOptions & IMergeViewOptions): Widget | null {

Check warning on line 90 in packages/nbdime/src/merge/widget/cell.ts

View check run for this annotation

Codecov / codecov/patch

packages/nbdime/src/merge/widget/cell.ts#L90

Added line #L90 was not covered by tests
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
Loading
Loading