Skip to content

Commit

Permalink
Vertical spacer outside of viewport are not included
Browse files Browse the repository at this point in the history
Fixes #693
  • Loading branch information
fcollonval committed Oct 10, 2023
1 parent faf3d14 commit edcd71d
Show file tree
Hide file tree
Showing 16 changed files with 134 additions and 55 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
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;
}
54 changes: 34 additions & 20 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 All @@ -1634,8 +1644,8 @@ export class MergeView extends Panel {
let processViewportAlignement = false;
const viewports = editors.map(e => ({
from: offsetToPos(e.state.doc, e.viewport.from),
to: offsetToPos(e.state.doc, e.viewport.to)
}))
to: offsetToPos(e.state.doc, e.viewport.to),
}));

const addStartSpacers = () => {
sumDeltas.forEach((delta, i) => {
Expand All @@ -1647,10 +1657,10 @@ export class MergeView extends Panel {
widget: new PaddingWidget(delta * lineHeight),
block: true,
side: -1,
})
}),
);
});
}
};

for (let alignment_ of linesToAlign) {
let alignment = this._showBase ? alignment_.slice(0, 3) : alignment_;
Expand All @@ -1666,15 +1676,17 @@ export class MergeView extends Panel {
let correctedDeltas = lineDeltas.map(line => line - minDelta);

const afterViewport = alignment.map((a, i) => a > viewports[i].to.line);
if(!afterViewport.some(after => !after)) {
if (!afterViewport.some(after => !after)) {
// All follow-up alignments are after the viewport => bail out
break;
}

const beforeViewport = alignment.map((a, i) => a < viewports[i].from.line)
const beforeViewport = alignment.map(
(a, i) => a < viewports[i].from.line,
);

if (beforeViewport.some(before => !before)){
if(!processViewportAlignement) {
if (beforeViewport.some(before => !before)) {
if (!processViewportAlignement) {
// First time we add spacer in the viewport => add initial spacers
addStartSpacers();
}
Expand Down Expand Up @@ -1707,13 +1719,15 @@ export class MergeView extends Panel {
}
});
} else {
correctedDeltas.forEach((delta, i) => { sumDeltas[i] += delta; });
}
correctedDeltas.forEach((delta, i) => {
sumDeltas[i] += delta;
});
}
}

if(!processViewportAlignement) {
if (!processViewportAlignement) {
// There are no spacer in the viewport, we still have to add top spacer
addStartSpacers()
addStartSpacers();
}

// Padding at the last line of the editor
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
Loading

0 comments on commit edcd71d

Please sign in to comment.