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.

Sign up for GitHub

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

Jump to bottom

3 Way Merge Editor Bugfixes #150709

Merged
merged 3 commits into from
May 31, 2022
Merged

3 Way Merge Editor Bugfixes #150709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
30aa8cc
Implements tri-state checkbox, some scrolling bug-fixing and checkbox…
hediet May 30, 2022
5b9978c
Fixes end-of-document line change bug.
hediet May 30, 2022
49ffe90
Fixes checkbox positioning at the document end.
hediet May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/vs/base/browser/ui/toggle/toggle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export class Toggle extends Widget {
readonly onKeyDown: Event<IKeyboardEvent> = this._onKeyDown.event;

private readonly _opts: IToggleOpts;
private _icon: CSSIcon | undefined;
readonly domNode: HTMLElement;

private _checked: boolean;
Expand All @@ -110,7 +111,8 @@ export class Toggle extends Widget {

const classes = ['monaco-custom-toggle'];
if (this._opts.icon) {
classes.push(...CSSIcon.asClassNameArray(this._opts.icon));
this._icon = this._opts.icon;
classes.push(...CSSIcon.asClassNameArray(this._icon));
}
if (this._opts.actionClassName) {
classes.push(...this._opts.actionClassName.split(' '));
Expand Down Expand Up @@ -174,6 +176,16 @@ export class Toggle extends Widget {
this.applyStyles();
}

setIcon(icon: CSSIcon | undefined): void {
if (this._icon) {
this.domNode.classList.remove(...CSSIcon.asClassNameArray(this._icon));
}
this._icon = icon;
if (this._icon) {
this.domNode.classList.add(...CSSIcon.asClassNameArray(this._icon));
}
}

width(): number {
return 2 /*margin left*/ + 2 /*border*/ + 2 /*padding*/ + 16 /* icon width */;
}
Expand Down
8 changes: 5 additions & 3 deletions src/vs/workbench/contrib/mergeEditor/browser/editorGutter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,11 @@ export class EditorGutter<
scrollTop +
lineHeight;

const bottom =
this._editor.getTopForLineNumber(gutterItem.range.endLineNumberExclusive) -
scrollTop;
const bottom = (
gutterItem.range.endLineNumberExclusive <= this._editor.getModel()!.getLineCount()
? this._editor.getTopForLineNumber(gutterItem.range.endLineNumberExclusive)
: this._editor.getTopForLineNumber(gutterItem.range.endLineNumberExclusive - 1) + lineHeight
) - scrollTop;

const height = bottom - top;

Expand Down
17 changes: 14 additions & 3 deletions src/vs/workbench/contrib/mergeEditor/browser/media/mergeEditor.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,19 @@
justify-content: center;
}

.merge-accept-gutter-marker .checkbox .accept-conflict-group.monaco-custom-toggle.monaco-checkbox {
margin: 0;
padding: 0;
.accept-conflict-group.monaco-custom-toggle {
height: 18px;
width: 18px;
border: 1px solid transparent;
border-radius: 3px;
margin-right: 0px;
margin-left: 0px;
padding: 0px;
opacity: 1;
background-size: 16px !important;
background-color: var(--vscode-checkbox-border);
}

.checkbox-background {
background: var(--vscode-editor-background);
}
146 changes: 82 additions & 64 deletions src/vs/workbench/contrib/mergeEditor/browser/mergeEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,48 +106,40 @@ export class MergeEditor extends EditorPane {
this._register(keepAlive(input1ResultMapping));
this._register(keepAlive(input2ResultMapping));

this._store.add(this.input1View.editor.onDidScrollChange(c => {
if (c.scrollTopChanged) {
reentrancyBarrier.runExclusively(() => {
const mapping = input1ResultMapping.get();
if (!mapping) {
return;
this._store.add(
this.input1View.editor.onDidScrollChange(
reentrancyBarrier.makeExclusive((c) => {
if (c.scrollTopChanged) {
const mapping = input1ResultMapping.get();
synchronizeScrolling(this.input1View.editor, this.inputResultView.editor, mapping, 1);
this.input2View.editor.setScrollTop(c.scrollTop, ScrollType.Immediate);
}
synchronizeScrolling(this.input1View.editor, this.inputResultView.editor, mapping, 1);
this.input2View.editor.setScrollTop(c.scrollTop, ScrollType.Immediate);
});
}
}));

this._store.add(this.input2View.editor.onDidScrollChange(c => {
if (c.scrollTopChanged) {
reentrancyBarrier.runExclusively(() => {
const mapping = input2ResultMapping.get();
if (!mapping) {
return;
}
synchronizeScrolling(this.input2View.editor, this.inputResultView.editor, mapping, 1);
this.input1View.editor.setScrollTop(c.scrollTop, ScrollType.Immediate);
});
}
}));
this._store.add(this.inputResultView.editor.onDidScrollChange(c => {
if (c.scrollTopChanged) {
reentrancyBarrier.runExclusively(() => {
const mapping = input1ResultMapping.get();
if (!mapping) {
return;
})
)
);
this._store.add(
this.input2View.editor.onDidScrollChange(
reentrancyBarrier.makeExclusive((c) => {
if (c.scrollTopChanged) {
const mapping = input2ResultMapping.get();
synchronizeScrolling(this.input2View.editor, this.inputResultView.editor, mapping, 1);
this.input1View.editor.setScrollTop(c.scrollTop, ScrollType.Immediate);
}
synchronizeScrolling(this.inputResultView.editor, this.input1View.editor, mapping, 2);

const mapping2 = input2ResultMapping.get();
if (!mapping2) {
return;
})
)
);
this._store.add(
this.inputResultView.editor.onDidScrollChange(
reentrancyBarrier.makeExclusive((c) => {
if (c.scrollTopChanged) {
const mapping1 = input1ResultMapping.get();
synchronizeScrolling(this.inputResultView.editor, this.input1View.editor, mapping1, 2);
const mapping2 = input2ResultMapping.get();
synchronizeScrolling(this.inputResultView.editor, this.input2View.editor, mapping2, 2);
}
synchronizeScrolling(this.inputResultView.editor, this.input2View.editor, mapping2, 2);
});
}
}));
})
)
);


// TODO@jrieken make this proper: add menu id and allow extensions to contribute
Expand Down Expand Up @@ -307,11 +299,11 @@ export class MergeEditor extends EditorPane {
}
}

function flip(value: 1 | 2): 1 | 2 {
return value === 1 ? 2 : 1;
}
function synchronizeScrolling(scrollingEditor: CodeEditorWidget, targetEditor: CodeEditorWidget, mapping: ModifiedBaseRange[] | undefined, sourceNumber: 1 | 2) {
if (!mapping) {
return;
}

function synchronizeScrolling(scrollingEditor: CodeEditorWidget, targetEditor: CodeEditorWidget, mapping: ModifiedBaseRange[], sourceNumber: 1 | 2) {
const visibleRanges = scrollingEditor.getVisibleRanges();
if (visibleRanges.length === 0) {
return;
Expand All @@ -322,30 +314,46 @@ function synchronizeScrolling(scrollingEditor: CodeEditorWidget, targetEditor: C
let sourceRange: LineRange;
let targetRange: LineRange;

const targetNumber = flip(sourceNumber);
const targetNumber = sourceNumber === 1 ? 2 : 1;

const firstBeforeSourceRange = firstBefore?.getInputRange(sourceNumber);
const firstBeforeTargetRange = firstBefore?.getInputRange(targetNumber);

if (firstBefore && firstBefore.getInputRange(sourceNumber).contains(topLineNumber)) {
sourceRange = firstBefore.getInputRange(sourceNumber);
targetRange = firstBefore.getInputRange(targetNumber);
} else if (firstBefore && firstBefore.getInputRange(sourceNumber).isEmpty && firstBefore.getInputRange(sourceNumber).startLineNumber === topLineNumber) {
sourceRange = firstBefore.getInputRange(sourceNumber).deltaEnd(1);
targetRange = firstBefore.getInputRange(targetNumber).deltaEnd(1);
if (firstBeforeSourceRange && firstBeforeSourceRange.contains(topLineNumber)) {
sourceRange = firstBeforeSourceRange;
targetRange = firstBeforeTargetRange!;
} else if (firstBeforeSourceRange && firstBeforeSourceRange.isEmpty && firstBeforeSourceRange.startLineNumber === topLineNumber) {
sourceRange = firstBeforeSourceRange.deltaEnd(1);
targetRange = firstBeforeTargetRange!.deltaEnd(1);
} else {
const delta = firstBefore ? firstBefore.getInputRange(targetNumber).endLineNumberExclusive - firstBefore.getInputRange(sourceNumber).endLineNumberExclusive : 0;
const delta = firstBeforeSourceRange ? firstBeforeTargetRange!.endLineNumberExclusive - firstBeforeSourceRange.endLineNumberExclusive : 0;
sourceRange = new LineRange(topLineNumber, 1);
targetRange = new LineRange(topLineNumber + delta, 1);
}

// sourceRange is not empty!
// sourceRange contains topLineNumber!

const resultStartTopPx = targetEditor.getTopForLineNumber(targetRange.startLineNumber);
const resultEndPx = targetEditor.getTopForLineNumber(targetRange.endLineNumberExclusive);

const sourceStartTopPx = scrollingEditor.getTopForLineNumber(sourceRange.startLineNumber);
const sourceEndPx = scrollingEditor.getTopForLineNumber(sourceRange.endLineNumberExclusive);

const factor = (scrollingEditor.getScrollTop() - sourceStartTopPx) / (sourceEndPx - sourceStartTopPx);
const factor = Math.min((scrollingEditor.getScrollTop() - sourceStartTopPx) / (sourceEndPx - sourceStartTopPx), 1);
const resultScrollPosition = resultStartTopPx + (resultEndPx - resultStartTopPx) * factor;
/*
console.log({
topLineNumber,
sourceRange: sourceRange.toString(),
targetRange: targetRange.toString(),
// resultStartTopPx,
// resultEndPx,
// sourceStartTopPx,
// sourceEndPx,
factor,
resultScrollPosition,
top: scrollingEditor.getScrollTop(),
});*/

targetEditor.setScrollTop(resultScrollPosition, ScrollType.Immediate);
}
Expand Down Expand Up @@ -468,7 +476,7 @@ class InputCodeEditorView extends CodeEditorView {
if (!model) { return []; }
return model.modifiedBaseRanges
.filter((r) => r.getInputDiffs(this.inputNumber).length > 0)
.map<MergeConflictData>((baseRange, idx) => ({
.map<ModifiedBaseRangeGutterItemInfo>((baseRange, idx) => ({
id: idx.toString(),
additionalHeightInPx: 0,
offsetInPx: 0,
Expand Down Expand Up @@ -497,25 +505,31 @@ class InputCodeEditorView extends CodeEditorView {
}
}

interface MergeConflictData extends IGutterItemInfo {
interface ModifiedBaseRangeGutterItemInfo extends IGutterItemInfo {
toggleState: IObservable<boolean | undefined>;
setState(value: boolean, tx: ITransaction | undefined): void;
}

class MergeConflictGutterItemView extends Disposable implements IGutterItemView<MergeConflictData> {
constructor(private item: MergeConflictData, target: HTMLElement) {
class MergeConflictGutterItemView extends Disposable implements IGutterItemView<ModifiedBaseRangeGutterItemInfo> {
constructor(private item: ModifiedBaseRangeGutterItemInfo, private readonly target: HTMLElement) {
super();

target.classList.add('merge-accept-gutter-marker');
target.classList.add(item.range.lineCount > 1 ? 'multi-line' : 'single-line');

// TODO: Tri-State-Toggle, localized title
const checkBox = new Toggle({ isChecked: false, title: 'Accept Merge', icon: Codicon.check, actionClassName: 'monaco-checkbox' });
// TODO: localized title
const checkBox = new Toggle({ isChecked: false, title: 'Accept Merge', icon: Codicon.check });
checkBox.domNode.classList.add('accept-conflict-group');

this._register(
autorun((reader) => {
const value = this.item.toggleState.read(reader);
checkBox.setIcon(
value === true
? Codicon.check
: value === false
? undefined
: Codicon.circleFilled
);
checkBox.checked = value === true;
}, 'Update Toggle State')
);
Expand All @@ -524,15 +538,19 @@ class MergeConflictGutterItemView extends Disposable implements IGutterItemView<
this.item.setState(checkBox.checked, undefined);
}));

target.appendChild($('div.background', {}, noBreakWhitespace));
target.appendChild($('div.checkbox', {}, checkBox.domNode));
target.appendChild(n('div.background', [noBreakWhitespace]).root);
target.appendChild(
n('div.checkbox', [n('div.checkbox-background', [checkBox.domNode])]).root
);
}

layout(top: number, height: number, viewTop: number, viewHeight: number): void {

this.target.classList.remove('multi-line');
this.target.classList.remove('single-line');
this.target.classList.add(height > 30 ? 'multi-line' : 'single-line');
}

update(baseRange: MergeConflictData): void {
update(baseRange: ModifiedBaseRangeGutterItemInfo): void {
this.item = baseRange;
}
}
Expand Down