diff --git a/nbdime-web/src/merge/manualedit.ts b/nbdime-web/src/merge/manualedit.ts new file mode 100644 index 00000000..c420f37e --- /dev/null +++ b/nbdime-web/src/merge/manualedit.ts @@ -0,0 +1,887 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. +'use strict'; + +import { + nbformat +} from '@jupyterlab/services'; + +import { + MergeDecision, buildDiffs, addSorted, pushPatchDecisionInPlace, + splitDiffStringPath +} from './decisions'; + +import { + labelSource +} from '../chunking'; + +import { + IDiffAddRange, IDiffRemoveRange, IDiffPatch, IDiffArrayEntry, + opAddRange, opRemoveRange, opPatch, + deepCopyDiff +} from '../diff/diffentries'; + +import { + IStringDiffModel +} from '../diff/model'; + +import { + getSubDiffByKey +} from '../diff/util'; + +import { + raw2Pos +} from '../diff/range'; + +import { + patchStringified, patch +} from '../patch'; + +import { + hasEntries, arraysEqual, splitLines, removeElement +} from '../common/util'; + +import { + CellMergeModel, DecisionStringDiffModel +} from './model'; + +import { + getPathForNewDecision +} from './util'; + + +/** + * Replace the content of an array with another, in-place + */ +function replaceArrayContent(array: T[], content: T[], start?: number, end?: number): void { + if (array === content && start === undefined && end === undefined) { + return; // No-op + } + start = start || 0; + if (end === undefined) { + end = array.length - start; + } + array.splice.apply(array, ([start, end - start] as any[]).concat(content)); +} + +function isCellSimpleDeletion(cell: CellMergeModel): boolean { + let ret = cell.local && cell.local.deleted && + (cell.remote === null || cell.remote.unchanged) || // Onesided deletion (other side unchanged) + cell.remote && cell.remote.deleted && + (cell.local === null || cell.local.unchanged) || // Onesided deletion (other side unchanged) + cell.local && cell.remote && + cell.local.deleted && cell.remote.deleted; // Deletion on both + return !!ret; +} + + +/** + * Get diffs in superset that precede + */ +function getPrecedingDiff(subset: AddRem[], superset: AddRem[]): AddRem[] { + if (!hasEntries(superset) || superset.length === subset.length) { + return []; + } + // Remove those we are sure are not a part of preceding diffs + let ret = superset.slice(0, subset.length); + for (let d of subset) { + let idx = ret.indexOf(d); + if (idx >= 0) { + // We assume diffs are sorted: + ret = ret.slice(0, idx); + break; + } + } + return ret; +} + + +/** + * Get the last diff, or, if the last diff is a removerange + * and two last diffs share the same key, the addrange. + */ +function getLastDiffAddRange(diff: AddRem[]): IDiffAddRange | null { + if (diff.length === 0) { + return null; + } + let e = diff[diff.length - 1]; + // If last op is removal, check if preceding op is addition + // on same key! + if (e.op === 'removerange' && + diff.length > 1 && diff[diff.length - 2].key === e.key) { + // Use preceding addrange as candidate + e = diff[diff.length - 2]; + } + return e.op === 'addrange' ? e : null; +} + +/** + * Whether both local and remote of the cell are unchanged. + * + * Note that this does not check whether the merge model is + * unchanged! + */ +function isCellUnchanged(cell: CellMergeModel): boolean { + let ret = cell.local && cell.local.unchanged && + cell.remote && cell.remote.unchanged; + return !!ret; +} + +export +interface IUpdateModelOptions { + model: IStringDiffModel; + + fullLines: string[]; + + baseLine: number; + + editCh: number; + + editLine: number; + + oldval: string[]; + + newval: string[]; +} + +type AddRem = IDiffAddRange | IDiffRemoveRange; + +/** + * Converts any patch ops to an addrange/removerange pair + */ +function convertPatchOps(diff: IDiffArrayEntry[], options: IUpdateModelOptions): AddRem[] { + // This function is optimized for maximal performance + // when there are no patch ops (most common case) + let anyPatch = false; + for (let e of diff) { + if (e.op === 'patch') { + anyPatch = true; + break; + } + } + if (!anyPatch) { + return diff as AddRem[]; + } + let lineoffset = 0; + let lines = splitLines(options.model.base!); + let ret: AddRem[] = []; + for (let i=0; i < diff.length; ++i) { + let e = diff[i]; + if (e.op === 'patch') { + let source = e.source; + if (!source && hasEntries(e.diff)) { + for (let d of e.diff) { + if (d.source) { + source = d.source; + break; + } + } + } + let patchedLine = patch( + lines[e.key + lineoffset], + [opPatch(0, e.diff)]); + if (i > 0 && diff[i - 1].key === e.key && + diff[i - 1].source === e.source) { + let d = diff[i - 1] as IDiffAddRange; + let vl = d.valuelist as string[]; + // Previous diff was also on this key, merge addranges + vl.push(patchedLine); + // Add removerange + let drr = opRemoveRange(e.key, 1); + drr.source = source; + ret.push(drr); + } else { + let dar = opAddRange(e.key, [patchedLine]); + let drr = opRemoveRange(e.key, 1); + dar.source = drr.source = source; + ret = ret.concat([dar, drr]); + } + } else { + ret.push(e); + if (e.op === 'addrange') { + lineoffset += e.valuelist.length; + } else { + lineoffset -= e.length; + } + } + } + return ret; +} + +function getPartials(diff: AddRem[], options: IUpdateModelOptions, startOffset: number, endOffset: number): { + before: string[], after: string[], linediff: number, key: number } { + let line = options.baseLine; + let ch = options.editCh; + let {oldval, newval} = options; + let lines: string[] = options.fullLines; + let key = line; + let linediff = 0; + + let lastAddedLine = newval[newval.length - 1]; + let lastDeletedLine = oldval[oldval.length - 1]; + + let partialStart = ch > 0; + let fullDeleteEnd = oldval.length > 1 && lastDeletedLine === ''; + let fullLineInsertions = ch === 0 && newval.length > 0 && lastAddedLine === ''; + + let before: string[] = ['']; + let after: string[] | null = null; + + // Check for overlap with first diff: + let firstDiff = diff[0]; + if (firstDiff && firstDiff.key <= line && firstDiff.op === 'addrange') { + let vlLine = options.editLine - (firstDiff.key + startOffset); + // Shift start forward: + key = firstDiff.key; + let vl = firstDiff.valuelist as string[]; + // Include any lines before edit: + before = vl.slice(0, vlLine); + if (!partialStart || vlLine < vl.length) { + // Add partial line + before.push( + partialStart ? vl[vlLine].slice(0, ch) : '' + ); + } else if (partialStart) { + // Addrange before current line (abutting) + // and partial edit of current (unedited) line + before.push(lines[options.editLine].slice(0, ch)); + // Indicate that one unedited line should be removed + linediff = 1; + } + } else if (partialStart) { + // Replace previously unedited line: + before = [lines[options.editLine].slice(0, ch)]; + // Indicate that one unedited line should be removed + linediff = 1; + } + + // Check for overlap with last diff: + let lastDiff = getLastDiffAddRange(diff); + if (lastDiff) { + let diffEditStart = lastDiff.key + startOffset + endOffset; + let matchLine = (options.editLine - diffEditStart) + oldval.length - 1; + if (matchLine >= 0 && matchLine < lastDiff.valuelist.length) { + let vl = lastDiff.valuelist as string[]; + // Figure out which bits to keep: + let cut = lastDeletedLine.length; + // If edit on one line, skip part before edit also + if (oldval.length === 1) { + cut += ch; + } + // Add partial line + after = [vl[matchLine].slice(cut)]; + // Include any lines after edit + after = after.concat(vl.slice(matchLine + 1)); + } + } + if (!after && !fullDeleteEnd && !fullLineInsertions) { + // Add remains of previously unedited line + let fullLine = lines[options.editLine + newval.length - 1]; + let cut = lastAddedLine.length; + if (newval.length === 1) { + cut += ch; + } + after = [fullLine.slice(cut)]; + // Indicate that an unedited line was replaced, but + // check if this line was also taken into account above. + if (oldval.length > 1) { + linediff += 1; + } else { + linediff = 1; + } + } + return {before, after: after || [''], linediff, key}; +} + +function joinPartials(a: string[], b: string[]): string[] { + let joined = a.concat(b.slice(1)); + let iend = a.length - 1; + joined[iend] = joined[iend].concat(b[0]); + return joined; +} + +/** + * Get addrange to represent inserted text as well as any overlapping additions + */ +function getAddOp(diff: AddRem[], options: IUpdateModelOptions, startOffset: number, endOffset: number): + { addop: IDiffAddRange | null, linediff: number} { + let newval = options.newval; + let newValuelist: string[]; + + let {before, after, linediff, key} = getPartials( + diff, options, startOffset, endOffset); + + newValuelist = joinPartials(before, newval); + newValuelist = joinPartials(newValuelist, after); + + if (newValuelist.length === 1 && newValuelist[0].length === 0) { + return {addop: null, linediff}; + } else if (newValuelist[newValuelist.length - 1].length === 0) { + // Check if we were adding newline to last line of text + // with missing newline: + let lines = options.fullLines; + if (options.editLine !== lines.length - 2 || + lines[lines.length - 1].length !== 0) { + // Not, so remove tail + newValuelist.pop(); + } + } + return {addop: opAddRange(key, newValuelist), linediff}; +} + +function getRemOp(diff: AddRem[], options: IUpdateModelOptions, extraLinediff: number): IDiffRemoveRange | null { + let oldval = options.oldval; + let line = options.baseLine; + let linediff = extraLinediff; + + if (oldval.length > 1 || oldval[0].length > 0) { + // Reduce the deletions to only those that were not + // previously added by diff: + let deletions = removeAddedDeletions(diff, options); + linediff += deletions.length - 1; + } + + // Set up removerange for all affected lines: + let remop = opRemoveRange(line, linediff); + + // Next, extend remove op to replace overlapping removes + mergeOverlappingRemoves(remop, diff); + + if (remop.length === 0) { + return null; + } + return remop; +} + +/** + * Remove deletions that were added through an addrange. + * + * All diffs passed should overlap at least partially with + * the edit! + * + * Returns options.oldval without those lines that were added by diff. + */ +function removeAddedDeletions(diff: AddRem[], options: IUpdateModelOptions): string[] { + let offset = 0; + let line = options.baseLine; + let ret = options.oldval.slice(); // Copy + let origLength = ret.length; + for (let e of diff) { + if (e.op !== 'addrange') { + continue; + } + // All passed diffs overlap at least partially! + let overlapStart = Math.max(e.key, line); + let overlapEnd = Math.min(e.key + e.valuelist.length, line + origLength); + let overlap = overlapEnd - overlapStart; + ret.splice(Math.max(e.key - line - offset, 0), overlap); + offset += overlap; + } + if (ret.length === 0) { + ret = ['']; + } + return ret; +} + +function mergeOverlappingRemoves(remop: IDiffRemoveRange, diff: AddRem[]): void { + for (let e of diff) { + if (e.op === 'removerange') { + // Extend end of diff + remop.length += e.length; + } + } +} + + +function findEditOverlap(diff: IDiffArrayEntry[], options: IUpdateModelOptions): number[] { + let editLength = options.oldval.length; + let {baseLine, editLine} = options; + let start = 0; + let end = diff.length; + let startOffset = 0; + let endOffset = 0; + let previousOffset = 0; + for (let i=0; i < diff.length; ++i) { + let e = diff[i]; + if (e.key < baseLine && (e.op !== 'addrange' || + e.key + startOffset + e.valuelist.length <= editLine)) { + // Before start of edit + ++start; + if (e.op !== 'patch') { + startOffset += e.op === 'addrange' ? e.valuelist.length : -e.length; + } + } else if (e.key + endOffset >= baseLine + editLength) { + // After end of edit + end = i; + break; + } else { + // Op within edit, adjust offset to find end + if (e.op === 'addrange') { + previousOffset = 0; + if (i + 1 < diff.length && diff[i + 1].key === e.key) { + // Next op is on same key (know/assume removerange) + // it should therefore also be within edit + let d = diff[++i] as IDiffRemoveRange; + endOffset -= d.length; + previousOffset -= d.length; + } + endOffset += e.valuelist.length; + previousOffset += e.valuelist.length; + } else if (e.op === 'removerange') { + endOffset -= e.length; + previousOffset = e.length; + } else { + previousOffset = 0; + if (i + 1 < diff.length && diff[i + 1].key === e.key) { + // Next op is on same key (know/assume addrange) + // it should therefore also be within edit + let d = diff[++i] as IDiffAddRange; + endOffset += d.valuelist.length; + previousOffset = d.valuelist.length; + } + } + } + } + endOffset -= previousOffset; + return [start, end, startOffset, endOffset]; +} + +/** + * Update a diff with an edit, in-place + */ +function updateDiff(diffToUpdate: IDiffArrayEntry[], options: IUpdateModelOptions, precedingDiff?: IDiffArrayEntry[]): void { + let diff = convertPatchOps(diffToUpdate, options); + + // First, figure out which part of the diff is overlapping with edit: + let [start, end, startOffset, endOffset] = findEditOverlap(diff, options); + let overlappingDiff = diff.slice(start, end); + if (precedingDiff) { + startOffset += findEditOverlap(precedingDiff, options)[2]; + } + + // All overlapping diffs should be replaced by at maximum + // a single addrange, and a single removerange + + // Find remains of any partial line deletions + let {addop, linediff} = getAddOp(overlappingDiff, options, startOffset, endOffset); + let remop = getRemOp(overlappingDiff, options, linediff); + + // Align start + if (addop && remop) { + addop.key = remop.key = Math.min(addop.key, remop.key); + } + + // Sanity check: + if (!addop && !remop) { + throw new Error( + 'An edit should result in at least one diff operation'); + } + + // Replace back in overlapping diff + let newop: AddRem[] = []; + if (addop) { + newop.push(addop); + } + if (remop) { + newop.push(remop); + } + replaceArrayContent(diff, newop, start, end); + replaceArrayContent(diffToUpdate, diff); +} + + +/** + * Updates an inserted cell. + * + * Inserted cells have a single action: + * side A: null + * side B: addrange with a single ICell + * custom: addrange with a single, edited ICell + */ +function updateInsertedCell(options: IUpdateModelOptions): void { + let model = options.model; + let cell = model.parent as CellMergeModel; + let path = getPathForNewDecision(model); + let subkey = path[1]; + if (cell.decisions.length !== 1) { + throw new Error('Unexpected length of model\'s decisions'); + } + if (model.additions.length !== 1) { + throw new Error('Unexpected length of added models additions field'); + } + + // Update additions: + let lines = options.fullLines; + // TODO: Split lines touched vs untouched, and then labelSource + // source as appropriate to give informative highlighting + model.additions[0].to.line = lines.length - 1; + model.additions[0].to.ch = lines[lines.length - 1].length; + + // Update decision: + let dec = cell.decisions[0]; + dec.action = 'custom'; + let diff = dec.customDiff; + if (!hasEntries(diff)) { + diff = hasEntries(dec.localDiff) ? dec.localDiff : dec.remoteDiff!; + dec.customDiff = diff = deepCopyDiff(diff); + } + // We can now modify diff in place to update decision + let cellVal = (diff[0] as IDiffAddRange).valuelist[0] as nbformat.ICell; + cellVal[subkey![0]] = options.fullLines.join(''); + + labelSource(diff, {decision: dec, action: 'custom'}); + model.additions[0].source = diff[0].source; +} + + +/** + * Updates a deleted cell. + * + * Deleted cell have a single action: + * side A: deleted, + * side B: unchanged + * custom: manual edits + * + * This cannot be changed, so we simply update the decision + * on cell level + */ +function updateDeletedCell(options: IUpdateModelOptions): void { + // Patch deleted model + // Update diff on decision + // Use diff to find additions/deletions + let model = options.model; + let cell = model.parent as CellMergeModel; + let path = getPathForNewDecision(model); + + let dec = cell.decisions[0]; + let diff: IDiffArrayEntry[]; + // Ensure we got a diff: + if (!hasEntries(dec.customDiff)) { + diff = []; + dec.customDiff = [opPatch(path[0][1], [opPatch(path[0][2], diff)])]; + } else { + let celldiff = (dec.customDiff[0] as IDiffPatch).diff!; + let subdiff = getSubDiffByKey(celldiff, path[0][2]); + if (subdiff !== null) { + diff = subdiff as IDiffArrayEntry[]; + } else { + diff = []; + celldiff.push(opPatch(path[0][2], diff)); + } + } + // Update diff with changes + updateDiff(diff, options); + labelSource(diff, {decision: dec, action: 'custom'}); + + // Update additions/deletions + let out = patchStringified(model.base || '', diff); + model.additions = raw2Pos(out.additions, out.remote); + model.deletions = raw2Pos(out.deletions, model.base || ''); + dec.action = 'custom'; +} + + +/** + * Updates an unchanged cell. + * + * Unchanged cells have no actions initially: + * local: unchanged + * remote: unchanged + * custom: unchanged OR multiple decisions with manual edits + */ +function updatedUnchangedCell(options: IUpdateModelOptions): void { + // All decisions are custom + patchPatchedModel(options, 'custom'); +} + + +/** + * Updates a patched cell. + * + * Pathced cells have more complex structure: + * local: deleted, patched or unchanged + * remote: deleted, patched or unchanged + * custom: patched, possibly with both manual edits and other changes + * + * Note that only one side can be deleted/unchanged at the same time. + */ +function updatedPatchedCell(options: IUpdateModelOptions): void { + // Decisions might not all be custom + patchPatchedModel(options, 'all'); +} + +/** + * Combines all decisions into the first decision + */ +function combineDecisions(decisions: MergeDecision[], diffs: 'all' | 'custom', base: any): void { + if (decisions.length < 2) { + return; + } + let target = decisions[0]; + target.customDiff = buildDiffs(base, decisions, 'merged') || []; + if (diffs === 'all') { + target.localDiff = buildDiffs(base, decisions, 'local') || []; + target.remoteDiff = buildDiffs(base, decisions, 'remote') || []; + } + target.action = 'custom'; + decisions.length = 1; +} + +function createPatchedModelDecision(options: IUpdateModelOptions, diff: AddRem[]): MergeDecision { + let model = options.model as DecisionStringDiffModel; + let path = getPathForNewDecision(model)[0]; + let dec = new MergeDecision(path, [], [], 'custom', false, diff); + dec.level = 3; + addSorted(model.decisions, dec, options.baseLine); + let cell = model.parent as CellMergeModel; + addSorted(cell.decisions, dec, options.baseLine); + return dec; +} + + +/** + * Assuming decisions that have already been split on chunks, + * this returns the base start and end indices of the diffs + * in the decision + */ +function getDecisionChunk(base: any, decision: MergeDecision): {baseStart: number, baseEnd: number} { + let line = splitDiffStringPath(base, decision.localPath)[1]; + if (hasEntries(line)) { + let lineIdx = line[0] as number; + // We have a decision that is a patch of a line + return {baseStart: lineIdx, baseEnd: lineIdx + 1}; + } + let boundaries: number[] = []; + for (let diff of decision.diffs) { + if (!hasEntries(diff)) { + continue; + } + let dd = diff as IDiffArrayEntry[]; + for (let e of dd) { + boundaries.push(e.key); + if (e.op === 'removerange') { + boundaries.push(e.key + e.length); + } else if (e.op === 'patch') { + boundaries.push(e.key + 1); + } + } + } + return {baseStart: Math.min(...boundaries), baseEnd: Math.max(...boundaries)}; +} + + +function ensureDecisionNotOnLine(decision: MergeDecision, base: any) { + let line = splitDiffStringPath(base, decision.localPath)[1]; + if (hasEntries(line)) { + if (line.length > 1) { + throw new Error('Unexpected path'); + } + pushPatchDecisionInPlace(decision, line); + } +} + + +function patchPatchedModel(options: IUpdateModelOptions, diffs: 'all' | 'custom') { + // Patch model + // Update remote + // Find overlapping decisions (arg specifies which diffs to consider) + // Add/replace custom diff (action -> custom) + // Add new decisions (on subkey) + let model = options.model as DecisionStringDiffModel; + + // Figure out which decisions overlap with edit: + let allDecisions = model.decisions; + let affectedDecisions: MergeDecision[] = []; + let baseLength = options.oldval.length; + let {baseLine} = options; + for (let md of allDecisions) { + let chunk = getDecisionChunk(model.base, md); + if (chunk.baseEnd < baseLine || chunk.baseStart > baseLine + baseLength) { + // No overlap + } else { + // Decision overlaps with edit + affectedDecisions.push(md); + } + } + + // Ensure we have one, and only one decision, and that it is on the right level/path: + if (affectedDecisions.length === 0) { + let dec = createPatchedModelDecision(options, []); + affectedDecisions.push(dec); + } else if (affectedDecisions.length > 1) { + // We merge all decisions that overlap + ensureDecisionNotOnLine(affectedDecisions[0], model.base); + let path = affectedDecisions[0].absolutePath; + let cell = model.parent as CellMergeModel; + for (let dec of affectedDecisions.slice(1)) { + // Fix: Check if any decisions have path to line + ensureDecisionNotOnLine(dec, model.base); + // TODO: Remove assertion once proven to work as expected: + if (!arraysEqual(path, dec.absolutePath)) { + throw new Error('Paths should be equal: "' + path + '", "' + dec.absolutePath + '"'); + } else if (diffs === 'custom' && (hasEntries(dec.localDiff) || hasEntries(dec.remoteDiff))) { + throw new Error('Expected only custom diffs!'); + } + // Remove decision from cell + model, as we merge + removeElement(model.decisions, dec); + removeElement(cell.decisions, dec); + } + combineDecisions(affectedDecisions, diffs, model.base); + } else { // affectedDecisions.length === 1 + let path = getPathForNewDecision(model)[0]; + ensureDecisionNotOnLine(affectedDecisions[0], model.base); + if (!arraysEqual(affectedDecisions[0].absolutePath, path)) { + throw new Error('Paths should be equal: "' + path + '", "' + affectedDecisions[0].absolutePath + '"'); + } + } + + let dec = affectedDecisions[0]; + if (diffs === 'all') { + let affectedDiff = buildDiffs(model.base, affectedDecisions, 'merged') || []; + if (!dec.customDiff) { + dec.customDiff = affectedDiff; + } else { + replaceArrayContent(dec.customDiff, affectedDiff); + } + // Set action to custom + dec.action = 'custom'; + } + if (dec.customDiff === null) { + throw new Error('Unexpected missing custom diff!'); + } + // Need to use all diffs so `updateDiff` can keep track of offsets + let allDiffs = buildDiffs(model.base, model.decisions, 'merged') as AddRem[] || []; + let precedingDiff = getPrecedingDiff(dec.customDiff as AddRem[], allDiffs); + updateDiff(dec.customDiff as IDiffArrayEntry[], options, precedingDiff); + if (!hasEntries(dec.customDiff)) { + // Diff has been modified to the point where it is equal to no changed + dec.action = 'base'; + } else { + labelSource(dec.customDiff, {decision: dec, action: 'custom'}); + } + + // Find overlapping decisions + // Add custom diff if needed + // Replace custom diff if existing + // Set action to custom + // Add new decisions for changes that do no overlap +} + + +/** + * Modify certain edits to use trailing newlines instead of leading + * + * Certain edits, e.g. adding a newline link this (bracketed part added): + * line1[\n + * line2]\n + * can better be treated as: + * line1\n + * [line2\n] + * + * Similarly (bracketed part deleted): + * line1[\n + * line2]\n + * can better be treated as: + * line1\n + * [line2\n] + * + * The end results are the same, but the diffs become simpler. + * + * Note that this needs to be called before base line is computed! + */ +export +function shiftAddRemoveLines(base: string[], change: CodeMirror.EditorChange) { + if (change.from.ch === 0) { + // Nothing to do here + return; + } + if (change.removed.length === 1 && change.removed[0] === '') { + // Nothing removed + // Check whether first inserted character is newline + if (change.text.length > 1 && change.text[0] === '') { + // and first subsequent character is newline + let trailingLine = base[change.to.line]; + if (trailingLine.length > change.to.ch && trailingLine[change.to.ch] === '\n') { + // Match, shift newline from head to tail + change.from.line += 1; + change.from.ch = 0; + change.text.push(change.text.shift()!); + } + } + } else if (change.text.length === 1 && change.text[0] === '') { + // Nothing added + // Check whether first removed character is newline + if (change.removed.length > 1 && change.removed[0] === '') { + // and first subsequent character is newline + let trailingLine = base[change.to.line]; + if (trailingLine.length > change.to.ch && trailingLine[change.to.ch] === '\n') { + // Match, shift newline from head to tail + change.from.line += 1; + change.from.ch = 0; + change.to.line += 1; + change.to.ch = 0; + change.removed.push(change.removed.shift()!); + } + } + } else { + // Both added and removed + // Check whether first removed character is newline + if (change.removed.length > 1 && change.removed[0] === '') { + // and first subsequent character is newline + let trailingLine = base[change.to.line]; + if (trailingLine.length > change.to.ch && trailingLine[change.to.ch] === '\n') { + // Match, shift newline from head to tail + change.from.line += 1; + change.from.ch = 0; + change.to.line += 1; + change.to.ch = 0; + change.text.push(change.text.shift()!); + change.removed.push(change.removed.shift()!); + } + } + } +} + +export +function updateModel(options: IUpdateModelOptions) { + let model = options.model; + let cell = model.parent as CellMergeModel; + // Update replaced value to include newlines as apropriate: + let oldval = options.oldval; + if (oldval.length > 1 || oldval[0].length > 0) { + for (let i=0; i < oldval.length - 1; ++i) { + if (oldval[i][oldval[i].length - 1] !== '\n') { + oldval[i] = oldval[i] + '\n'; + } + } + } + let newval = options.newval; + if (newval.length > 1 || newval[0].length > 0) { + for (let i=0; i < newval.length - 1; ++i) { + newval[i] = newval[i] + '\n'; + } + } + + // Update remote: + model.remote = options.fullLines.join(''); + + // If edited, cell should not be marked for deletion + if (cell.deleteCell) { + cell.deleteCell = false; + } + if (model.added) { + // Inserted cell + updateInsertedCell(options); + } else if (isCellSimpleDeletion(cell)) { + // Simple deletion + updateDeletedCell(options); + } else if (isCellUnchanged(cell)) { + // Local and remote both unchanged, all decisions are custom + updatedUnchangedCell(options); + } else { + updatedPatchedCell(options); + } + if (model instanceof DecisionStringDiffModel) { + model.invalidate(); + } +} diff --git a/nbdime-web/src/merge/util.ts b/nbdime-web/src/merge/util.ts new file mode 100644 index 00000000..17024644 --- /dev/null +++ b/nbdime-web/src/merge/util.ts @@ -0,0 +1,72 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. +'use strict'; + + +import { + IModel, CellDiffModel +} from '../diff/model'; + +import { + DecisionPath +} from './decisions'; + +import { + NotebookMergeModel, CellMergeModel +} from './model'; + + +export +function getRootModel(model: IModel): IModel { + while (model.parent !== null){ + model = model.parent; + } + return model; +} + + +export +function getPathForNewDecision(model: IModel): [DecisionPath, DecisionPath | null] { + let parent = model.parent; + if (parent === null) { + // We are a root level model + return [[], null]; + } + if (parent instanceof NotebookMergeModel) { + if (model === parent.metadata) { + return [['metadata'], null]; + } + let cm = model as CellMergeModel; + if (cm.base === null) { + // Inserted cell + return [['cells'], []]; + } else { + let idx = parent.base.cells.indexOf(cm.base); + if (idx === -1) { + throw new Error('Invalid model'); + } + return [['cells', idx], null]; + } + } + let parentPath = getPathForNewDecision(parent); + // If parent is inserted cell, this will pick subpath: + let subpath = parentPath[1] || parentPath[0]; + if (parent instanceof CellDiffModel) { + if (model === parent.source) { + subpath.push('source'); + } else if (model === parent.metadata) { + subpath.push('metadata'); + } + return parentPath; + // Do not support editing on outputs, so excluded + } else if (parent instanceof CellMergeModel) { + if (model === parent.merged.source) { + subpath.push('source'); + } else if (model === parent.merged.metadata) { + subpath.push('metadata'); + } + return parentPath; + // Do not support editing on outputs, so excluded + } + throw new Error('Could not find path for model'); +} diff --git a/nbdime-web/test/src/merge/manualedit.spec.ts b/nbdime-web/test/src/merge/manualedit.spec.ts new file mode 100644 index 00000000..071a1e2c --- /dev/null +++ b/nbdime-web/test/src/merge/manualedit.spec.ts @@ -0,0 +1,702 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. +'use strict'; + +import expect = require('expect.js'); + +import { + notebookStub, codeCellStub +} from '../testutil'; + +import { + stripSource +} from '../../../src/diff/util'; + +import { + shallowCopy +} from '../../../src/common/util'; + +import { + opPatch, opAddRange, opRemoveRange, IDiffAddRange, + IDiffPatchArray +} from '../../../src/diff/diffentries'; + +import { + IMergeDecision +} from '../../../src/merge/decisions'; + +import { + NotebookMergeModel +} from '../../../src/merge/model'; + +import { + updateModel +} from '../../../src/merge/manualedit'; + + + + + +describe('merge', () => { + + describe('updateModel', () => { + + describe('unchanged cell', () => { + + it('should update a simple line insertion', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + nbcontent.cells = [cell]; + let nbmodel = new NotebookMergeModel(nbcontent, []); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', 'jkl\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: [''], + newval: ['ghi', ''] + }); + expect(model.remote).to.be('abcdef\nghi\njkl\n'); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells', 0, 'source']); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect(diff).to.eql([ + opAddRange(1, ['ghi\n']) + ]); + }); + + it('should update a simple line deletion', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + nbcontent.cells = [cell]; + let nbmodel = new NotebookMergeModel(nbcontent, []); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: ['jkl', ''], + newval: [''] + }); + expect(model.remote).to.be('abcdef\n'); + expect(model.additions).to.be.empty(); + expect(model.deletions.length).to.be(1); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells', 0, 'source']); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect(diff).to.eql([ + opRemoveRange(1, 1) + ]); + }); + + }); + + + describe('inserted cell', () => { + + it('should update a simple line addition', () => { + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opAddRange(0, [cell])], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(notebookStub, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', 'jkl\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: [''], + newval: ['ghi', ''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\nghi\njkl\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect((diff[0] as IDiffAddRange).valuelist[0]).to.eql(expectedCell); + expect(model.deletions).to.be.empty(); + expect(model.additions.length).to.be(1); + expect(model.additions[0].from).to.eql({line: 0, ch: 0}); + expect(model.additions[0].to).to.eql({line: 3, ch: 0}); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should update a simple character addition', () => { + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\ngh\njkl\n'; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opAddRange(0, [cell])], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(notebookStub, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', 'jkl\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: [''], + newval: ['i'] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\nghi\njkl\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect((diff[0] as IDiffAddRange).valuelist[0]).to.eql(expectedCell); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should update a simple line deletion', () => { + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opAddRange(0, [cell])], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(notebookStub, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: ['jkl', ''], + newval: [''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect((diff[0] as IDiffAddRange).valuelist[0]).to.eql(expectedCell); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should update a simple character deletion', () => { + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\nghi\njkl\n'; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opAddRange(0, [cell])], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(notebookStub, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'gh\n', 'jkl\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: ['i'], + newval: [''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\ngh\njkl\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect((diff[0] as IDiffAddRange).valuelist[0]).to.eql(expectedCell); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should update a two subsequent line deletions', () => { + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\nmnp\n'; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'remote_diff': [opAddRange(0, [cell])], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(notebookStub, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'mnp\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: ['jkl', ''], + newval: [''] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: ['mnp', ''], + newval: [''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect((diff[0] as IDiffAddRange).valuelist[0]).to.eql(expectedCell); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should update a simple line replacement', () => { + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opAddRange(0, [cell])], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(notebookStub, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: ['jkl', ''], + newval: ['ghi', ''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\nghi\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + expect((diff[0] as IDiffAddRange).valuelist[0]).to.eql(expectedCell); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + }); + + + describe('deleted cell', () => { + + it('should update a simple line addition', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + nbcontent.cells = [cell]; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opRemoveRange(0, 1)], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', 'jkl\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: [''], + newval: ['ghi', ''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\nghi\njkl\n'; + expect(model.remote).to.be(expectedCell.source); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + expect(diff).to.eql([ + opAddRange(1, ['ghi\n']) + ]); + }); + + it('should update a simple line deletion', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\nghi\njkl\n'; + nbcontent.cells = [cell]; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opRemoveRange(0, 1)], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'jkl\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: ['ghi', ''], + newval: [''] + }); + expect(model.remote).to.be('abcdef\njkl\n'); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + expect(diff[0]).to.eql(opRemoveRange(1, 1)); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should update a deletion on a previously edited model', () => { + // This test adds an initial custom diff, whose changes are + // subsequently all deleted + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\nmnpq\nuv\n'; + nbcontent.cells = [cell]; + // Set up initial deleted cell model + let decisions: IMergeDecision[] = [{ + common_path: ['cells'], + local_diff: [opRemoveRange(0, 1)], + action: 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let cellModel = nbmodel.cells[0]; + // Simulate diff after previous manual edit + let dec = cellModel.decisions[0]; + dec.customDiff = [opPatch(0, [opPatch('source', [ + opAddRange(1, ['ghi\n']), + opRemoveRange(1, 1), + opPatch(2, [ + opAddRange(2, 'o'), + opRemoveRange(2, 1) + ]), + opAddRange(3, ['rst\n']) + ])])]; + dec.action = 'custom'; + // Perform new edit: + let model = cellModel.merged.source; + // Value at this point: + // 'abcdef\nghi\nmnoq\nrst\nuv\n' + updateModel({ + model: model, + fullLines: ['abcv\n', ''], + baseLine: 0, + editLine: 0, + editCh: 3, + oldval: ['def', 'ghi', 'mnoq', 'rst', 'u'], + newval: [''] + }); + expect(model.remote).to.be('abcv\n'); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + expect(diff).to.eql([ + opAddRange(0, ['abcv\n']), + opRemoveRange(0, 5) + ]); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should update a partial deletion on a previously edited model', () => { + // This test adds an initial custom diff, whose changes are + // subsequently partially deleted + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\nmnpq\nuv\n'; + nbcontent.cells = [cell]; + // Set up initial deleted cell model + let decisions: IMergeDecision[] = [{ + common_path: ['cells'], + local_diff: [opRemoveRange(0, 1)], + action: 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let cellModel = nbmodel.cells[0]; + // Simulate diff after previous manual edit + let dec = cellModel.decisions[0]; + dec.customDiff = [opPatch(0, [opPatch('source', [ + opAddRange(1, ['ghi\n']), + opRemoveRange(1, 1), + opPatch(2, [ + opAddRange(2, 'o'), + opRemoveRange(2, 1) + ]), + opAddRange(3, ['rst\n']) + ])])]; + dec.action = 'custom'; + // Perform new edit: + let model = cellModel.merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghq\n', 'rst\n', 'uv\n', ''], + baseLine: 1, + editLine: 1, + editCh: 2, + oldval: ['i', 'mno'], + newval: [''] + }); + expect(model.remote).to.be('abcdef\nghq\nrst\nuv\n'); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + expect(diff).to.eql([ + opAddRange(1, ['ghq\n']), + opRemoveRange(1, 2), + opAddRange(3, ['rst\n']) + ]); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should handle two subsequent insertions', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + nbcontent.cells = [cell]; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opRemoveRange(0, 1)], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', 'jkl\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: [''], + newval: ['ghi', ''] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', 'xyz\n', 'jkl\n', ''], + baseLine: 1, + editLine: 2, + editCh: 0, + oldval: [''], + newval: ['xyz', ''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\nghi\nxyz\njkl\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + expect(diff).to.eql([ + opAddRange(1, ['ghi\n', 'xyz\n']) + ]); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should handle a newline insertion at end of line', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + nbcontent.cells = [cell]; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opRemoveRange(0, 1)], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abcdef\n', '\n', 'jkl\n', ''], + baseLine: 0, + editLine: 0, + editCh: 6, + oldval: [''], + newval: ['', ''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\n\njkl\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + // Ideal: [opAddRange(1, ['\n'])] + expect(diff).to.eql([ + opAddRange(0, ['abcdef\n', '\n']), + opRemoveRange(0, 1) + ]); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should handle a newline insertion in the middle of a line', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\njkl\n'; + nbcontent.cells = [cell]; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opRemoveRange(0, 1)], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let model = nbmodel.cells[0].merged.source; + updateModel({ + model: model, + fullLines: ['abc\n', 'def\n', 'jkl\n', ''], + baseLine: 0, + editLine: 0, + editCh: 3, + oldval: [''], + newval: ['', ''] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abc\ndef\njkl\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + expect(diff).to.eql([ + opAddRange(0, ['abc\n', 'def\n']), + opRemoveRange(0, 1) + ]); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + it('should handle a sequence of character and line insertions', () => { + let nbcontent = shallowCopy(notebookStub); + let cell = shallowCopy(codeCellStub); + cell.source = 'abcdef\nghi\njkl\nmnop\n'; + nbcontent.cells = [cell]; + let decisions: IMergeDecision[] = [{ + 'common_path': ['cells'], + 'local_diff': [opRemoveRange(0, 1)], + 'action': 'local' + }]; + let nbmodel = new NotebookMergeModel(nbcontent, decisions); + let model = nbmodel.cells[0].merged.source; + // Insert new line + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', '\n', 'jkl\n', 'mnop\n', ''], + baseLine: 2, + editLine: 2, + editCh: 0, + oldval: [''], + newval: ['', ''] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', '1\n', 'jkl\n', 'mnop\n', ''], + baseLine: 2, + editLine: 2, + editCh: 0, + oldval: [''], + newval: ['1'] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', '13\n', 'jkl\n', 'mnop\n', ''], + baseLine: 2, + editLine: 2, + editCh: 1, + oldval: [''], + newval: ['3'] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', 'ghi\n', '123\n', 'jkl\n', 'mnop\n', ''], + baseLine: 2, + editLine: 2, + editCh: 1, + oldval: [''], + newval: ['2'] + }); + // Insert new line + updateModel({ + model: model, + fullLines: ['abcdef\n', '\n', 'ghi\n', '123\n', 'jkl\n', 'mnop\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: [''], + newval: ['', ''] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', '4\n', 'ghi\n', '123\n', 'jkl\n', 'mnop\n', ''], + baseLine: 1, + editLine: 1, + editCh: 0, + oldval: [''], + newval: ['4'] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', '45\n', 'ghi\n', '123\n', 'jkl\n', 'mnop\n', ''], + baseLine: 1, + editLine: 1, + editCh: 1, + oldval: [''], + newval: ['5'] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', '45\n', '6ghi\n', '123\n', 'jkl\n', 'mnop\n', ''], + baseLine: 1, + editLine: 2, + editCh: 0, + oldval: [''], + newval: ['6'] + }); + updateModel({ + model: model, + fullLines: ['abcdef\n', '45\n', '67ghi\n', '123\n', 'jkl\n', 'mnop\n', ''], + baseLine: 1, + editLine: 2, + editCh: 1, + oldval: [''], + newval: ['7'] + }); + let expectedCell = shallowCopy(codeCellStub); + expectedCell.source = 'abcdef\n45\n67ghi\n123\njkl\nmnop\n'; + expect(model.remote).to.be(expectedCell.source); + let diff = stripSource(nbmodel.cells[0].decisions[0].customDiff)!; + diff = (diff[0] as IDiffPatchArray).diff!; + diff = (diff[0] as IDiffPatchArray).diff!; + expect(diff).to.eql([ + opAddRange(1, ['45\n', '67ghi\n']), + opRemoveRange(1, 1), + opAddRange(2, ['123\n']) + ]); + expect(nbmodel.cells[0].deleteCell).to.be(false); + expect(nbmodel.cells[0].decisions.length).to.be(1); + expect(nbmodel.cells[0].decisions[0].absolutePath).to.eql(['cells']); + }); + + }); + + + describe('patched cell', () => { + + it('should handle a pure addition after a replace range with non-zero line diff', () => { + // TODO: Write + }); + + }); + + }); + +}); diff --git a/nbdime-web/test/src/testutil.ts b/nbdime-web/test/src/testutil.ts new file mode 100644 index 00000000..eff8cafb --- /dev/null +++ b/nbdime-web/test/src/testutil.ts @@ -0,0 +1,47 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + +import { + nbformat +} from '@jupyterlab/services'; + + +export +const notebookStub: nbformat.INotebookContent = { + 'cells': [ + ], + 'metadata': { + 'kernelspec': { + 'display_name': 'Python 2', + 'language': 'python', + 'name': 'python2' + }, + 'language_info': { + 'codemirror_mode': { + 'name': 'ipython', + 'version': 2 + }, + 'file_extension': '.py', + 'mimetype': 'text/x-python', + 'name': 'python', + 'nbconvert_exporter': 'python', + 'pygments_lexer': 'ipython2', + 'version': '2.7.11' + }, + 'orig_nbformat': 4 + }, + 'nbformat': 4, + 'nbformat_minor': 0 +} + +export +const codeCellStub: nbformat.ICodeCell = { + cell_type: 'code', + execution_count: 0, + metadata: { + trusted: true + }, + source: [''], + outputs: [] +} + diff --git a/packages/nbdime/src/chunking/decisionchunking.ts b/packages/nbdime/src/chunking/decisionchunking.ts index 3d75117f..ec6aa44d 100644 --- a/packages/nbdime/src/chunking/decisionchunking.ts +++ b/packages/nbdime/src/chunking/decisionchunking.ts @@ -88,7 +88,7 @@ function splitDiffsOnBoundaries(diffs: IDiffArrayEntry[], boundaries: number[]): * Make list of chunks on the form (j, k, diffs0, diffs1, ..., diffsN), * where `j` and `k` are line numbers in the base, and the `diffsX` * entries are subsets from `diffs` that are part of the chunk. - + * * Because the diff entries have been split on the union of * begin/end boundaries of all diff entries, the keys of * diff entries on each side will always match a boundary diff --git a/packages/nbdime/src/chunking/diffchunking.ts b/packages/nbdime/src/chunking/diffchunking.ts index abb167dd..4e1f1530 100644 --- a/packages/nbdime/src/chunking/diffchunking.ts +++ b/packages/nbdime/src/chunking/diffchunking.ts @@ -16,7 +16,7 @@ import { } from '../merge/decisions'; import { - valueIn, shallowCopy + valueIn, shallowCopy, unique } from '../common/util'; @@ -166,6 +166,7 @@ class Chunker { } this.chunks.push(current); } + current.sources = current.sources.filter(unique); this.editOffset += isAddition ? -linediff : linediff; } @@ -222,6 +223,7 @@ class Chunker { this.chunks.push(current); } this._currentGhost = current; + current.sources = current.sources.filter(unique); // this._doAdd(range, isAddition); } @@ -285,6 +287,7 @@ function lineToNormalChunks(lineChunks: Chunk[]): Chunk[] { current = shallowCopy(c); } } + current.sources = current.sources.filter(unique); } if (current !== null) { ret.push(current); diff --git a/packages/nbdime/src/common/mergeview.ts b/packages/nbdime/src/common/mergeview.ts index 69c9fd0c..2cfbb596 100644 --- a/packages/nbdime/src/common/mergeview.ts +++ b/packages/nbdime/src/common/mergeview.ts @@ -22,7 +22,7 @@ import { } from '../diff/model'; import { - DecisionStringDiffModel + DecisionStringDiffModel, CellMergeModel } from '../merge/model'; import { @@ -38,13 +38,17 @@ import { } from './editor'; import { - valueIn, hasEntries, splitLines, copyObj + valueIn, hasEntries, splitLines, copyObj, removeElement } from './util'; import { NotifyUserError } from './exceptions'; +import { + updateModel, shiftAddRemoveLines +} from '../merge/manualedit'; + const PICKER_SYMBOL = '\u27ad'; @@ -155,6 +159,33 @@ function createNbdimeMergeView( } +/** + * Replace a range in an array of lines, with another + * + * Note: `lines` is assumed to have newlines at the ends, while + * newVal not. + */ +function replaceCodeMirrorRange(lines: string[], from: CodeMirror.Position, to: CodeMirror.Position, newVal: string[]): void { + // Add preceding and trailing characters to newVal, as well as newlines + let firstLine = lines[from.line]; + let lastLine = lines[to.line]; + + newVal = newVal.slice(); // copy + for (let i=0; i < newVal.length; ++i) { + if (i === 0) { + newVal[i] = firstLine.slice(0, from.ch) + newVal[i]; + } + if (i >= newVal.length - 1) { + newVal[i] = newVal[i] + lastLine.slice(to.ch); + } else { + newVal[i] = newVal[i] + '\n'; + } + } + + lines.splice(from.line, to.line - from.line + 1, ...newVal); +} + + /** * Used by MergeView to show diff in a string diff model */ @@ -203,25 +234,48 @@ class DiffView { syncModel() { if (this.modelInvalid()) { let edit = this.ownEditor; + let updatedLineChunks = this.model.getLineChunks(); + let updatedChunks = lineToNormalChunks(updatedLineChunks); + if (this.model.remote === edit.getValue()) { + // Nothing to do except update chunks + this.lineChunks = updatedLineChunks; + this.chunks = updatedChunks; + return; + } let cursor = edit.getDoc().getCursor(); let newLines = splitLines(this.model.remote!); let start = edit.getDoc().firstLine(); let last = edit.getDoc().lastLine(); let end = last; + let updatedEnd = last; + let cumulativeOffset = 0; for (let range of this.collapsedRanges) { let baseLine = range.line; end = getMatchingEditLine(baseLine, this.chunks); - if (end !== start) { - edit.getDoc().replaceRange(newLines.slice(start, end - 1).join(''), CodeMirror.Pos(start, 0), CodeMirror.Pos(end - 1, 0)); + updatedEnd = getMatchingEditLine(baseLine, updatedChunks); + let offset = updatedEnd - end; + if (end !== start || offset !== 0) { + edit.getDoc().replaceRange( + newLines.slice(start + cumulativeOffset, updatedEnd + cumulativeOffset - 1).join(''), + CodeMirror.Pos(start, 0), + CodeMirror.Pos(end - 1, 0), + 'syncModel' + ); } + cumulativeOffset += offset; start = end + range.size; } if (start < last) { - edit.getDoc().replaceRange(newLines.slice(start, end).join(''), CodeMirror.Pos(start, 0), CodeMirror.Pos(end, 0)); + edit.getDoc().replaceRange( + newLines.slice(start, last).join(''), + CodeMirror.Pos(start, 0), + CodeMirror.Pos(last, 0), + 'syncModel' + ); } this.ownEditor.getDoc().setCursor(cursor); - this.lineChunks = this.model.getLineChunks(); - this.chunks = lineToNormalChunks(this.lineChunks); + this.lineChunks = updatedLineChunks; + this.chunks = updatedChunks; } } @@ -279,7 +333,7 @@ class DiffView { checkSync(self.ownEditor); self.updating = false; } - function setDealign(fast: boolean | CodeMirror.Editor) { + function setDealign(fast: boolean | CodeMirror.Editor, mode?: 'full') { let upd = false; for (let dv of self.baseEditor.state.diffViews) { upd = upd || dv.updating; @@ -288,9 +342,9 @@ class DiffView { return; } self.dealigned = true; - set(fast === true); + set(fast === true, mode); } - function set(fast: boolean) { + function set(fast: boolean, mode?: 'full') { let upd = false; for (let dv of self.baseEditor.state.diffViews) { upd = upd || dv.updating || dv.updatingFast; @@ -302,14 +356,36 @@ class DiffView { if (fast === true) { self.updatingFast = true; } - debounceChange = window.setTimeout(update, fast === true ? 20 : 250); + debounceChange = window.setTimeout( + update.bind(self, mode), fast === true ? 20 : 250); } - function change(_cm: CodeMirror.Editor, change: CodeMirror.EditorChangeLinkedList) { - if (self.model instanceof DecisionStringDiffModel) { - self.model.invalidate(); + function change(_cm: CodeMirror.Editor, change: CodeMirror.EditorChange) { + let userEdit = !valueIn(change.origin, ['setValue', 'syncModel']); + if (userEdit) { + // Edited by hand! + let fullLines = splitLines(self.model.remote!); + shiftAddRemoveLines(fullLines, change); + let baseLine = getMatchingBaseLine(change.from.line, self.lineChunks); + // Update lines with changes + replaceCodeMirrorRange(fullLines, change.from, change.to, change.text); + updateModel({ + model: self.model, + baseLine, + fullLines, + editCh: change.from.ch, + editLine: change.from.line, + oldval: change.removed, + newval: change.text + }); + } + if (!(self.model instanceof DecisionStringDiffModel)) { + // TODO: Throttle? + self.lineChunks = self.model.getLineChunks(); + self.chunks = lineToNormalChunks(self.lineChunks); } // Update faster when a line was added/removed - setDealign(change.text.length - 1 !== change.to.line - change.from.line); + setDealign(change.text.length - 1 !== change.to.line - change.from.line, + userEdit ? 'full' : undefined); } function checkSync(cm: CodeMirror.Editor) { if (self.model.remote !== cm.getValue()) { @@ -330,6 +406,11 @@ class DiffView { return update; } + validate(): boolean { + return this.model instanceof DecisionStringDiffModel && + this.model.invalid; + } + protected modelInvalid(): boolean { return this.model instanceof DecisionStringDiffModel && this.model.invalid; @@ -352,14 +433,36 @@ class DiffView { for (let s of ss) { s.decision.action = s.action; } - } else if (instance === this.baseEditor) { + } else if (this.type === 'merge' && instance === this.baseEditor) { for (let s of ss) { s.decision.action = 'base'; - if (hasEntries(s.decision.customDiff)) { - s.decision.customDiff = []; + } + } + for (let i=ss.length - 1; i >= 0; --i) { + let s = ss[i]; + if (this.type === 'merge' && hasEntries(s.decision.customDiff)) { + // Custom diffs are cleared on pick, + // as there is no way to re-pick them + s.decision.customDiff = []; + // If decision is now empty, remove decision: + if (!hasEntries(s.decision.localDiff) && + !hasEntries(s.decision.remoteDiff)) { + // Remove decision: + let model = this.model as DecisionStringDiffModel; + let cell = model.parent as CellMergeModel; + removeElement(model.decisions, s.decision); + removeElement(cell.decisions, s.decision); + // Remove source + ss.splice(i, 1); } } } + if (ss.length === 0) { + // All decisions empty, remove picker + // In these cases, there should only be one picker, on base + // so simply remove the one we have here + instance.setGutterMarker(line, GUTTER_PICKER_CLASS, null); + } } else if (gutter === GUTTER_CONFLICT_CLASS) { for (let s of ss) { s.decision.conflict = false; @@ -492,6 +595,18 @@ class DiffView { (picker as any).sources = sources; picker.classList.add(GUTTER_PICKER_CLASS); editor.setGutterMarker(line, GUTTER_PICKER_CLASS, picker); + } else if (editor === self.baseEditor) { + for (let s of sources) { + if (s.decision.action === 'custom' && + !hasEntries(s.decision.localDiff) && + !hasEntries(s.decision.remoteDiff)) { + // We have a custom decision, add picker on base only! + let picker = elt('div', PICKER_SYMBOL, classes.gutter); + (picker as any).sources = sources; + picker.classList.add(GUTTER_PICKER_CLASS); + editor.setGutterMarker(line, GUTTER_PICKER_CLASS, picker); + } + } } else if (conflict && editor === self.ownEditor) { // Add conflict markers on editor, if conflicted let conflictMarker = elt('div', CONFLICT_MARKER, ''); @@ -668,9 +783,34 @@ function getMatchingEditLineLC(toMatch: Chunk, chunks: Chunk[]): number { return toMatch.baseTo; } +/** + * From a line in edit, find the matching line in base by chunks. + */ +function getMatchingBaseLine(editLine: number, chunks: Chunk[]): number { + let offset = 0; + // Start values correspond to either the start of the chunk, + // or the start of a preceding unmodified part before the chunk. + // It is the difference between these two that is interesting. + for (let i = 0; i < chunks.length; i++) { + let chunk = chunks[i]; + if (chunk.remoteTo > editLine && chunk.remoteFrom <= editLine) { + // If remote range is larger than base range, clip + if (editLine + offset > chunk.baseTo) { + return chunk.baseTo; + } + break; + } + if (chunk.remoteFrom > editLine) { + break; + } + offset = chunk.baseTo - chunk.remoteTo; + } + return editLine + offset; +} + /** - * Find which line numbers align which each other, in the + * Find which line numbers align with each other, in the * set of DiffViews. The returned array is of the format: * * [ aligned line #1:[Edit line number, (DiffView#1 line number, DiffView#2 line number,) ...], diff --git a/packages/nbdime/src/common/util.ts b/packages/nbdime/src/common/util.ts index 040e543b..56c88cfd 100644 --- a/packages/nbdime/src/common/util.ts +++ b/packages/nbdime/src/common/util.ts @@ -4,12 +4,12 @@ export interface DeepCopyableObject { - [key: string]: DeepCopyableValue | undefined; + [key: string]: any | undefined; prototype?: DeepCopyableObject; } export -type DeepCopyableValue = DeepCopyableObject | any[] | string | number | boolean | null; +type DeepCopyableValue = DeepCopyableObject | DeepCopyableObject[] | string | number | boolean | null; /** * Check whether a value is in an array. @@ -303,3 +303,24 @@ function buildSelect(options: string[], select?: HTMLSelectElement): HTMLSelectE } return select; } + + +/** + * Extend one array with another + */ +export +function extendArray(a: any[], b: any[] | null): void { + if (!b || b.length === 0) { + return; + } + a.splice.apply(a, [a.length, 0].concat(b)); +} + +/** + * Remove first matching element from an array + */ +export +function removeElement(array: any[], element: any): void { + let i = array.indexOf(element); + array.splice(i, 1); +} diff --git a/packages/nbdime/src/diff/diffentries.ts b/packages/nbdime/src/diff/diffentries.ts index ae035a2f..8d64622b 100644 --- a/packages/nbdime/src/diff/diffentries.ts +++ b/packages/nbdime/src/diff/diffentries.ts @@ -3,7 +3,7 @@ 'use strict'; import { - valueIn + valueIn, deepCopy } from '../common/util'; import { @@ -250,3 +250,32 @@ function validateObjectOp(base: any, entry: IDiffEntry, keys: string[]): void { throw new Error('Invalid op: ' + op); } } + +/** + * Deep copy of diff, but with same source references + */ +export function deepCopyDiff(diff: null): null; +export function deepCopyDiff(diff: T[]): T[]; +export function deepCopyDiff(diff: T[] | null): T[] | null; +export function deepCopyDiff(diff: IDiffEntry[] | null): IDiffEntry[] | null { + if (!diff) { + return diff; + } + let copy: IDiffEntry[] = []; + for (let e of diff) { + let cp: any = {}; + cp.key = e.key; + cp.op = e.op; + cp.source = e.source; + for (let sub of ['valuelist', 'value', 'length']) { + if (e.hasOwnProperty(sub)) { + cp[sub] = deepCopy((e as any)[sub]); + } + } + if (e.op === 'patch') { + cp.diff = deepCopyDiff(e.diff); + } + copy.push(cp); + } + return copy; +} diff --git a/packages/nbdime/src/diff/model/cell.ts b/packages/nbdime/src/diff/model/cell.ts index e989cdb6..12a8298d 100644 --- a/packages/nbdime/src/diff/model/cell.ts +++ b/packages/nbdime/src/diff/model/cell.ts @@ -22,6 +22,10 @@ import { getSubDiffByKey, getDiffEntryByKey } from '../util'; +import { + IModel +} from './common'; + import { IStringDiffModel, StringDiffModel, createDirectStringDiffModel, createPatchStringDiffModel, setMimetypeFromCellType @@ -40,8 +44,10 @@ import { /** * Diff model for individual Notebook Cells */ -export class CellDiffModel { - constructor(source: IStringDiffModel, +export +class CellDiffModel implements IModel { + constructor(parent: IModel, + source: IStringDiffModel, metadata: IStringDiffModel, outputs: OutputDiffModel[] | null, executionCount: ImmutableDiffModel | null, @@ -51,6 +57,7 @@ export class CellDiffModel { this.outputs = outputs; this.executionCount = executionCount; this.cellType = cellType; + this.parent = parent; if (outputs === null && cellType === 'code') { throw new NotifyUserError('Invalid code cell, missing outputs!'); } @@ -90,6 +97,10 @@ export class CellDiffModel { */ cellType: string; + /** + * Parent model + */ + readonly parent: IModel; /** * Whether the cell has remained unchanged @@ -161,7 +172,10 @@ export class CellDiffModel { export function createPatchedCellDiffModel( - base: nbformat.ICell, diff: IDiffEntry[] | null, nbMimetype: string): CellDiffModel { + parent: IModel, + base: nbformat.ICell, + diff: IDiffEntry[] | null, + nbMimetype: string): CellDiffModel { let source: StringDiffModel | null = null; let metadata: StringDiffModel | null = null; let outputs: OutputDiffModel[] | null = null; @@ -169,16 +183,16 @@ function createPatchedCellDiffModel( let subDiff = getSubDiffByKey(diff, 'source'); if (subDiff) { - source = createPatchStringDiffModel(base.source, subDiff); + source = createPatchStringDiffModel(parent, base.source, subDiff); } else { - source = createDirectStringDiffModel(base.source, base.source); + source = createDirectStringDiffModel(parent, base.source, base.source); } setMimetypeFromCellType(source, base, nbMimetype); subDiff = getSubDiffByKey(diff, 'metadata'); metadata = subDiff ? - createPatchStringDiffModel(base.metadata as JSONObject, subDiff) : - createDirectStringDiffModel(base.metadata as JSONObject, base.metadata as JSONObject); + createPatchStringDiffModel(parent, base.metadata as JSONObject, subDiff) : + createDirectStringDiffModel(parent, base.metadata as JSONObject, base.metadata as JSONObject); if (nbformat.isCode(base)) { let outputsBase = base.outputs; @@ -186,68 +200,76 @@ function createPatchedCellDiffModel( if (outputsDiff) { // Outputs patched outputs = makeOutputModels( - outputsBase, null, outputsDiff); + parent, outputsBase, null, outputsDiff); } else { // Outputs unchanged outputs = makeOutputModels( - outputsBase, outputsBase); + parent, outputsBase, outputsBase); } let execBase = base.execution_count; let execDiff = getDiffEntryByKey(diff, 'execution_count') as IDiffReplace | null; // Pass base as remote, which means fall back to unchanged if no diff: - executionCount = createImmutableModel(execBase, execBase, execDiff); + executionCount = createImmutableModel(parent, execBase, execBase, execDiff); } - return new CellDiffModel(source, metadata, outputs, executionCount, base.cell_type); + return new CellDiffModel(parent, source, metadata, outputs, executionCount, base.cell_type); } export function createUnchangedCellDiffModel( - base: nbformat.ICell, nbMimetype: string): CellDiffModel { - let source = createDirectStringDiffModel(base.source, base.source); + parent: IModel, + base: nbformat.ICell, + nbMimetype: string): CellDiffModel { + let source = createDirectStringDiffModel(parent, base.source, base.source); setMimetypeFromCellType(source, base, nbMimetype); - let metadata = createDirectStringDiffModel(base.metadata as JSONObject, base.metadata as JSONObject); + let metadata = createDirectStringDiffModel(parent, base.metadata as JSONObject, base.metadata as JSONObject); let outputs: OutputDiffModel[] | null = null; let executionCount: ImmutableDiffModel | null = null; if (nbformat.isCode(base)) { - outputs = makeOutputModels(base.outputs, + outputs = makeOutputModels( + parent, + base.outputs, base.outputs); let execBase = base.execution_count; - executionCount = createImmutableModel(execBase, execBase); + executionCount = createImmutableModel(parent, execBase, execBase); } else { // markdown or raw cell } - return new CellDiffModel(source, metadata, outputs, executionCount, base.cell_type); + return new CellDiffModel(parent, source, metadata, outputs, executionCount, base.cell_type); } export function createAddedCellDiffModel( - remote: nbformat.ICell, nbMimetype: string): CellDiffModel { - let source = createDirectStringDiffModel(null, remote.source); + parent: IModel, + remote: nbformat.ICell, + nbMimetype: string): CellDiffModel { + let source = createDirectStringDiffModel(parent, null, remote.source); setMimetypeFromCellType(source, remote, nbMimetype); - let metadata = createDirectStringDiffModel(null, remote.metadata as JSONObject); + let metadata = createDirectStringDiffModel(parent, null, remote.metadata as JSONObject); let outputs: OutputDiffModel[] | null = null; let executionCount: ImmutableDiffModel | null = null; if (nbformat.isCode(remote)) { outputs = makeOutputModels( - null, remote.outputs); - executionCount = createImmutableModel(null, remote.execution_count); + parent, null, remote.outputs); + executionCount = createImmutableModel(parent, null, remote.execution_count); } - return new CellDiffModel(source, metadata, outputs, executionCount, remote.cell_type); + return new CellDiffModel(parent, source, metadata, outputs, executionCount, remote.cell_type); } export function createDeletedCellDiffModel( - base: nbformat.ICell, nbMimetype: string): CellDiffModel { - let source = createDirectStringDiffModel(base.source, null); + parent: IModel, + base: nbformat.ICell, + nbMimetype: string): CellDiffModel { + let source = createDirectStringDiffModel(parent, base.source, null); setMimetypeFromCellType(source, base, nbMimetype); - let metadata = createDirectStringDiffModel(base.metadata as JSONObject, null); + let metadata = createDirectStringDiffModel(parent, base.metadata as JSONObject, null); let outputs: OutputDiffModel[] | null = null; let executionCount: ImmutableDiffModel | null = null; if (nbformat.isCode(base)) { - outputs = makeOutputModels(base.outputs, null); + outputs = makeOutputModels(parent, base.outputs, null); let execBase = base.execution_count; - executionCount = createImmutableModel(execBase, null); + executionCount = createImmutableModel(parent, execBase, null); } - return new CellDiffModel(source, metadata, outputs, executionCount, base.cell_type); + return new CellDiffModel(parent, source, metadata, outputs, executionCount, base.cell_type); } diff --git a/packages/nbdime/src/diff/model/common.ts b/packages/nbdime/src/diff/model/common.ts index 85833d66..9fa453cc 100644 --- a/packages/nbdime/src/diff/model/common.ts +++ b/packages/nbdime/src/diff/model/common.ts @@ -3,6 +3,18 @@ 'use strict'; +/** + * Describes any model. + */ +export +interface IModel { + /** + * Parent model + */ + readonly parent: IModel | null; +} + + /** * Describes a model whose view can be collapsible. * @@ -30,7 +42,7 @@ interface ICollapsibleModel { * Base interface for diff models. */ export -interface IDiffModel extends ICollapsibleModel { +interface IDiffModel extends ICollapsibleModel, IModel { /** * Is diff no-op? */ diff --git a/packages/nbdime/src/diff/model/immutable.ts b/packages/nbdime/src/diff/model/immutable.ts index 63435154..f4d9ae0c 100644 --- a/packages/nbdime/src/diff/model/immutable.ts +++ b/packages/nbdime/src/diff/model/immutable.ts @@ -3,7 +3,7 @@ 'use strict'; import { - IDiffModel + IDiffModel, IModel } from './common'; import { @@ -27,11 +27,13 @@ class ImmutableDiffModel implements IDiffModel { * `collapsible` and `collapsed` both defaults to false. */ constructor( - base: ImmutableValue | undefined, - remote: ImmutableValue | undefined, - collapsible?: boolean, - header?: string, - collapsed?: boolean) { + parent: IModel, + base: ImmutableValue | undefined, + remote: ImmutableValue | undefined, + collapsible?: boolean, + header?: string, + collapsed?: boolean) { + this.parent = parent; this.base = base; this.remote = remote; @@ -54,6 +56,8 @@ class ImmutableDiffModel implements IDiffModel { return this.remote === undefined; } + parent: IModel; + base: ImmutableValue | undefined; remote: ImmutableValue | undefined; @@ -76,23 +80,23 @@ class ImmutableDiffModel implements IDiffModel { * @returns {ImmutableDiffModel} */ export -function createImmutableModel(base: ImmutableValue | undefined, remote: ImmutableValue | undefined, diff?: IDiffImmutableObjectEntry | null): ImmutableDiffModel { +function createImmutableModel(parent: IModel, base: ImmutableValue | undefined, remote: ImmutableValue | undefined, diff?: IDiffImmutableObjectEntry | null): ImmutableDiffModel { if (!diff) { - return new ImmutableDiffModel(base, remote); + return new ImmutableDiffModel(parent, base, remote); } else if (diff.op === 'add') { if (base !== undefined) { throw new Error('Invalid diff op on immutable value'); } - return new ImmutableDiffModel(base, diff.value); + return new ImmutableDiffModel(parent, base, diff.value); } else if (diff.op === 'remove') { if (base === undefined) { throw new Error('Invalid diff op on immutable value'); } - return new ImmutableDiffModel(base, undefined); + return new ImmutableDiffModel(parent, base, undefined); } else { // diff.op === 'replace' if (base === undefined) { throw new Error('Invalid diff op on immutable value'); } - return new ImmutableDiffModel(base, diff.value); + return new ImmutableDiffModel(parent, base, diff.value); } } diff --git a/packages/nbdime/src/diff/model/notebook.ts b/packages/nbdime/src/diff/model/notebook.ts index 6bbdc5a5..591dfe06 100644 --- a/packages/nbdime/src/diff/model/notebook.ts +++ b/packages/nbdime/src/diff/model/notebook.ts @@ -14,6 +14,10 @@ import { getSubDiffByKey } from '../util'; +import { + IModel +} from './common'; + import { IStringDiffModel, createPatchStringDiffModel } from './string'; @@ -28,7 +32,8 @@ import { /** * Diff model for a Jupyter Notebook */ -export class NotebookDiffModel { +export +class NotebookDiffModel implements IModel { /** * Create a new NotebookDiffModel from a base notebook and a list of diffs. @@ -40,7 +45,7 @@ export class NotebookDiffModel { // Process global notebook metadata field let metaDiff = getSubDiffByKey(diff, 'metadata'); if (base.metadata && metaDiff) { - this.metadata = createPatchStringDiffModel(base.metadata, metaDiff); + this.metadata = createPatchStringDiffModel(this, base.metadata, metaDiff); } else { this.metadata = null; } @@ -71,7 +76,7 @@ export class NotebookDiffModel { // diff is sorted on index, so take any preceding cells as unchanged: for (let i=take; i < index; i++) { - let cell = createUnchangedCellDiffModel(base.cells[i], this.mimetype); + let cell = createUnchangedCellDiffModel(this, base.cells[i], this.mimetype); this.cells.push(cell); this.chunkedCells.push([cell]); } @@ -86,7 +91,7 @@ export class NotebookDiffModel { if (e.op === 'addrange') { // One or more inserted/added cells: for (let ei of e.valuelist) { - let cell = createAddedCellDiffModel(ei as nbformat.ICell, this.mimetype); + let cell = createAddedCellDiffModel(this, ei as nbformat.ICell, this.mimetype); this.cells.push(cell); currentChunk.push(cell); } @@ -95,7 +100,7 @@ export class NotebookDiffModel { // One or more removed/deleted cells: skip = e.length; for (let i=index; i < index + skip; i++) { - let cell = createDeletedCellDiffModel(base.cells[i], this.mimetype); + let cell = createDeletedCellDiffModel(this, base.cells[i], this.mimetype); this.cells.push(cell); currentChunk.push(cell); } @@ -106,7 +111,7 @@ export class NotebookDiffModel { this.chunkedCells.push(currentChunk); } // A cell has changed: - let cell = createPatchedCellDiffModel(base.cells[index], e.diff, this.mimetype); + let cell = createPatchedCellDiffModel(this, base.cells[index], e.diff, this.mimetype); this.cells.push(cell); currentChunk.push(cell); skip = 1; @@ -119,7 +124,7 @@ export class NotebookDiffModel { } // Take unchanged values at end for (let i=take; i < base.cells.length; i++) { - let cell = createUnchangedCellDiffModel(base.cells[i], this.mimetype); + let cell = createUnchangedCellDiffModel(this, base.cells[i], this.mimetype); this.cells.push(cell); this.chunkedCells.push([cell]); } @@ -146,4 +151,9 @@ export class NotebookDiffModel { * location optionally can be shown side by side. */ chunkedCells: CellDiffModel[][]; + + /** + * Notebook models do not have a parent, so this is always null + */ + parent: IModel | null = null; } diff --git a/packages/nbdime/src/diff/model/output.ts b/packages/nbdime/src/diff/model/output.ts index c3983589..2dfb2b1b 100644 --- a/packages/nbdime/src/diff/model/output.ts +++ b/packages/nbdime/src/diff/model/output.ts @@ -18,6 +18,10 @@ import { RenderableDiffModel } from './renderable'; +import { + IModel +} from './common'; + import { IStringDiffModel } from './string'; @@ -108,7 +112,8 @@ class OutputDiffModel extends RenderableDiffModel { * deleted entries. */ export -function makeOutputModels(base: nbformat.IOutput[] | null, +function makeOutputModels(parent: IModel, + base: nbformat.IOutput[] | null, remote: nbformat.IOutput[] | null, diff?: IDiffArrayEntry[] | null) : OutputDiffModel[] { let models: OutputDiffModel[] = []; @@ -118,7 +123,7 @@ function makeOutputModels(base: nbformat.IOutput[] | null, } // Cell deleted for (let o of base) { - models.push(new OutputDiffModel(o, null)); + models.push(new OutputDiffModel(parent, o, null)); } } else if (base === null) { if (remote === null) { @@ -126,12 +131,12 @@ function makeOutputModels(base: nbformat.IOutput[] | null, } // Cell added for (let o of remote) { - models.push(new OutputDiffModel(null, o)); + models.push(new OutputDiffModel(parent, null, o)); } } else if (remote === base) { // All entries unchanged for (let o of base) { - models.push(new OutputDiffModel(o, o)); + models.push(new OutputDiffModel(parent, o, o)); } } else if (diff) { // Entries patched, remote will be null @@ -141,25 +146,25 @@ function makeOutputModels(base: nbformat.IOutput[] | null, let index = d.key; for (let o of base.slice(consumed, index)) { // Add unchanged entries - models.push(new OutputDiffModel(o, o)); + models.push(new OutputDiffModel(parent, o, o)); } if (d.op === 'addrange') { // Entries added for (let o of d.valuelist) { - models.push(new OutputDiffModel(null, o)); + models.push(new OutputDiffModel(parent, null, o)); } skip = 0; } else if (d.op === 'removerange') { // Entries removed let len = d.length; for (let i = index; i < index + len; i++) { - models.push(new OutputDiffModel(base[i], null)); + models.push(new OutputDiffModel(parent, base[i], null)); } skip = len; } else if (d.op === 'patch') { // Entry changed models.push(new OutputDiffModel( - base[index], null, d.diff)); + parent, base[index], null, d.diff)); skip = 1; } else { throw new Error('Invalid diff operation: ' + d); @@ -168,7 +173,7 @@ function makeOutputModels(base: nbformat.IOutput[] | null, } for (let o of base.slice(consumed)) { // Add unchanged entries - models.push(new OutputDiffModel(o, o)); + models.push(new OutputDiffModel(parent, o, o)); } } else { throw new Error('Invalid arguments to makeOutputModels()'); diff --git a/packages/nbdime/src/diff/model/renderable.ts b/packages/nbdime/src/diff/model/renderable.ts index 950d1b28..975b315f 100644 --- a/packages/nbdime/src/diff/model/renderable.ts +++ b/packages/nbdime/src/diff/model/renderable.ts @@ -23,7 +23,7 @@ import { } from '../../patch'; import { - IDiffModel + IDiffModel, IModel } from './common'; import { @@ -42,12 +42,14 @@ import { export abstract class RenderableDiffModel implements IDiffModel { constructor( + parent: IModel, base: T | null, remote: T | null, diff?: IDiffEntry[] | null) { if (!remote && !base) { throw new Error('Either remote or base value need to be given'); } + this.parent = parent; this.base = base; if (!remote && diff) { this.remote = patch(base!, diff) as T; @@ -109,9 +111,9 @@ abstract class RenderableDiffModel implements IDiffModel { this.diff; let model: IStringDiffModel | null = null; if (this.unchanged || this.added || this.deleted || !diff) { - model = createDirectStringDiffModel(base, remote); + model = createDirectStringDiffModel(this.parent, base, remote); } else { - model = createPatchStringDiffModel(base, diff); + model = createPatchStringDiffModel(this.parent, base, diff); } model.mimetype = key || 'application/json'; model.collapsible = this.collapsible; @@ -120,6 +122,11 @@ abstract class RenderableDiffModel implements IDiffModel { return model; } + /** + * Parent model + */ + parent: IModel; + /** * Base value */ diff --git a/packages/nbdime/src/diff/model/string.ts b/packages/nbdime/src/diff/model/string.ts index f2ba030d..c58905f3 100644 --- a/packages/nbdime/src/diff/model/string.ts +++ b/packages/nbdime/src/diff/model/string.ts @@ -27,7 +27,7 @@ import { } from '../../patch'; import { - IDiffModel + IDiffModel, IModel } from './common'; @@ -100,13 +100,15 @@ class StringDiffModel implements IStringDiffModel { * Collapsible and collapsed both defaults to false. */ constructor( - public base: string | null, - public remote: string | null, - additions: DiffRangeRaw[], - deletions: DiffRangeRaw[], - collapsible?: boolean, - header?: string, - collapsed?: boolean) { + parent: IModel, + public base: string | null, + public remote: string | null, + additions: DiffRangeRaw[], + deletions: DiffRangeRaw[], + collapsible?: boolean, + header?: string, + collapsed?: boolean) { + this.parent = parent; if (base === null) { console.assert(deletions.length === 0); this.deletions = []; @@ -155,6 +157,8 @@ class StringDiffModel implements IStringDiffModel { return this.remote === null; } + readonly parent: IModel; + collapsible: boolean; collapsibleHeader: string; startCollapsed: boolean; @@ -333,11 +337,14 @@ namespace StringDiffModel { * rules. */ export -function createPatchStringDiffModel(base: string | JSONObject | JSONArray, diff: IDiffEntry[]) : StringDiffModel { +function createPatchStringDiffModel( + parent: IModel, + base: string | JSONObject | JSONArray, + diff: IDiffEntry[]) : StringDiffModel { console.assert(!!diff, 'Patch model needs diff.'); let baseStr = stringifyAndBlankNull(base); let out = patchStringified(base, diff); - return new StringDiffModel(baseStr, out.remote, out.additions, out.deletions); + return new StringDiffModel(parent, baseStr, out.remote, out.additions, out.deletions); } @@ -349,7 +356,10 @@ function createPatchStringDiffModel(base: string | JSONObject | JSONArray, diff: * unchanged content. */ export -function createDirectStringDiffModel(base: JSONValue | null, remote: JSONValue | null): StringDiffModel { +function createDirectStringDiffModel( + parent: IModel, + base: JSONValue | null, + remote: JSONValue | null): StringDiffModel { let baseStr: string | null = stringifyAndBlankNull(base); let remoteStr: string | null = stringifyAndBlankNull(remote); let additions: DiffRangeRaw[] = []; @@ -370,7 +380,7 @@ function createDirectStringDiffModel(base: JSONValue | null, remote: JSONValue | throw new Error('Invalid arguments to createDirectStringDiffModel(). ' + 'Either base or remote should be null, or they should be equal!'); } - return new StringDiffModel(baseStr, remoteStr, additions, deletions); + return new StringDiffModel(parent, baseStr, remoteStr, additions, deletions); } diff --git a/packages/nbdime/src/diff/util.ts b/packages/nbdime/src/diff/util.ts index e1661de5..350830ea 100644 --- a/packages/nbdime/src/diff/util.ts +++ b/packages/nbdime/src/diff/util.ts @@ -7,11 +7,7 @@ import { } from '../common/util'; import { - ChunkSource -} from '../chunking'; - -import { - IDiffEntry, IDiffArrayEntry, IDiffPatch, IDiffAddRange, IDiffRemoveRange, + IDiffEntry, IDiffArrayEntry, IDiffPatch, opAddRange, opRemoveRange, validateSequenceOp } from './diffentries'; @@ -73,21 +69,6 @@ function validateStringDiff(base: string[], entry: IDiffArrayEntry, lineToChar: } } - -function areSourcesCompatible(a: ChunkSource | undefined, b: ChunkSource | undefined): boolean { - if (!a || !b || a === b) { - return true; - } - if (a.decision === b.decision) { - // 'either' sources can be combined with local or remote - if (a.action === 'either' && b.action !== 'custom' || - b.action === 'either' && a.action !== 'custom') { - return true; - } - } - return false; -} - /** * Remove the merge source indicator from a diff (returns a copy). */ @@ -113,81 +94,6 @@ function stripSource(diff: IDiffEntry[] | null): IDiffEntry[] | null { return ret; } -/** - * Check whether existing collection of diff ops shares a key with the new - * diffop, and if they also have the same op type. - */ -function overlaps(existing: IDiffArrayEntry[], newv: IDiffArrayEntry): number { - if (existing.length < 1) { - return -1; - } - for (let i=0; i < existing.length; ++i) { - let e = existing[i]; - if (!areSourcesCompatible(e.source, newv.source)) { - continue; - } - if (e.op === newv.op) { - if (e.op === 'removerange') { - // Since patch ops of lines come before line add/remove - // (and patch ops can contain character removals), - // we need to check order: - let first = e.key <= newv.key ? e : newv as IDiffRemoveRange; - let last = e.key <= newv.key ? newv as IDiffRemoveRange : e; - if (first.key + first.length >= last.key) { - // Overlapping deletes - // Above check is open ended to allow for sanity check here: - if (first.key + first.length !== last.key) { - throw new Error('Overlapping delete diff ops: ' + - 'Two operation remove same characters!'); - } - return i; - } - } else if (e.key === newv.key) { - // Found a match - return i; - } else if (e.op === 'addrange') { - // Check if there is a removerange after the - // addrange that will cause the two addranges - // to abut. - if (i < existing.length - 1) { - let other = existing[i + 1]; - if (other.op === 'removerange' && - other.key === e.key && - other.key + other.length === newv.key) { - return i; - } - } - } - } - } - return -1; -} - - -/** - * Combines two ops into a new one that does the same - */ -function combineOps(a: IDiffArrayEntry, b: IDiffArrayEntry): IDiffAddRange | IDiffRemoveRange { - let combined: IDiffArrayEntry; - if (b.op === 'addrange') { - combined = opAddRange(a.key, (a as IDiffAddRange).valuelist); - // valuelist can also be string, but string also has concat: - combined.valuelist = (combined.valuelist as any[]).concat(b.valuelist as any[]); - } else if (b.op === 'removerange') { - if (a.op !== 'removerange') { - throw new Error('Cannot combine operations: ' + a + ', ' + b); - } - combined = opRemoveRange(a.key, a.length + b.length); - } else { - throw new Error('Invalid string lines op to combine: ' + b); - } - combined.source = a.source; - if (!areSourcesCompatible(a.source, b.source)) { - throw new Error('Cannot combine diff ops with incompatible sources in one string line'); - } - return combined; -} - /** * Translates a diff of strings split by str.splitlines() to a diff of the @@ -211,12 +117,7 @@ function flattenStringDiff(val: string[] | string, diff: IDiffArrayEntry[]): IDi for (let p of pdiff) { let d = shallowCopy(p); d.key += lineOffset; - let idx = overlaps(flattened, d); - if (idx >= 0) { - flattened[idx] = combineOps(flattened[idx], d); - } else { - flattened.push(d); - } + flattened.push(d); } } } else { @@ -231,13 +132,7 @@ function flattenStringDiff(val: string[] | string, diff: IDiffArrayEntry[]): IDi lineToChar[idx] - lineOffset); } d.source = e.source; - - let idx = overlaps(flattened, d); - if (idx >= 0) { - flattened[idx] = combineOps(flattened[idx], d); - } else { - flattened.push(d); - } + flattened.push(d); } } // Finally, sort on key (leaving equal items in original order) diff --git a/packages/nbdime/src/merge/decisions.ts b/packages/nbdime/src/merge/decisions.ts index 8aed2a37..ea690c81 100644 --- a/packages/nbdime/src/merge/decisions.ts +++ b/packages/nbdime/src/merge/decisions.ts @@ -380,10 +380,11 @@ function resolveCommonPaths(decisions: MergeDecision[]) { for (let md of decisions) { let diffs = md.diffs; let path = md.absolutePath || []; - let popped: {diffs: DiffCollection, key: string | number} | null = null; - while (popped = popPath(diffs, true)) { + let popped = popPath(diffs, true); + while (popped) { path.push(popped.key); diffs = popped.diffs; + popped = popPath(diffs, true); } md.absolutePath = path; md.diffs = diffs; @@ -500,6 +501,7 @@ function resolveAction(base: any, decision: MergeDecision): IDiffEntry[] { * * Returns a tuple of path and any line key. */ +export function splitDiffStringPath(base: any, path: DecisionPath): [DecisionPath, DecisionPath | null] { for (let i = 0; i < path.length; ++i) { @@ -710,6 +712,19 @@ function buildDiffs(base: any, decisions: MergeDecision[], which: 'local' | 'rem export function pushPatchDecision(decision: MergeDecision, prefix: DecisionPath): MergeDecision { let dec = new MergeDecision(decision); + pushPatchDecisionInPlace(dec, prefix); + return dec; +} + + +/** + * Move a path prefix in a merge decision from `common_path` to the diffs. + * + * This is done by wrapping the diffs in nested patch ops. + */ +export +function pushPatchDecisionInPlace(decision: MergeDecision, prefix: DecisionPath): void { + let dec = decision; // We need to start with inner most key to nest correctly, so reverse: for (let key of prefix.slice().reverse()) { if (dec.absolutePath.length === 0) { @@ -728,7 +743,6 @@ function pushPatchDecision(decision: MergeDecision, prefix: DecisionPath): Merge dec.remoteDiff = rd ? [opPatch(key, dec.remoteDiff)] : null; dec.customDiff = cd ? [opPatch(key, dec.customDiff)] : null; } - return dec; } diff --git a/packages/nbdime/src/merge/model/cell.ts b/packages/nbdime/src/merge/model/cell.ts index d26e7b2a..39719019 100644 --- a/packages/nbdime/src/merge/model/cell.ts +++ b/packages/nbdime/src/merge/model/cell.ts @@ -20,7 +20,7 @@ import { } from '../../diff/util'; import { - CellDiffModel, + CellDiffModel, IModel, createAddedCellDiffModel, createDeletedCellDiffModel, createPatchedCellDiffModel, createUnchangedCellDiffModel, OutputDiffModel, makeOutputModels, ImmutableDiffModel, @@ -60,6 +60,7 @@ import { * decisions that patch the cell. */ function createPatchedCellDecisionDiffModel( + parent: IModel, base: nbformat.ICell, decisions: MergeDecision[], local: CellDiffModel | null, remote: CellDiffModel | null, mimetype: string): @@ -76,12 +77,14 @@ function createPatchedCellDecisionDiffModel( } let source = new DecisionStringDiffModel( + parent, base.source, filterDecisions(decisions, ['source'], 2), [local ? local.source : null, remote ? remote.source : null]); setMimetypeFromCellType(source, base, mimetype); let metadata = new DecisionStringDiffModel( + parent, base.metadata, filterDecisions(decisions, ['metadata'], 2), [local ? local.metadata : null, remote ? remote.metadata : null]); @@ -99,7 +102,7 @@ function createPatchedCellDecisionDiffModel( } else { merged = outputBase; } - outputs = makeOutputModels(outputBase, merged, mergedDiff); + outputs = makeOutputModels(parent, outputBase, merged, mergedDiff); } let execBase = base.execution_count; let cellDecs = filterDecisions(decisions, ['cells'], 0, 2); @@ -111,13 +114,13 @@ function createPatchedCellDecisionDiffModel( let mergeExecDiff = buildDiffs(base, [dec], 'merged') as IDiffImmutableObjectEntry[] | null; let execDiff = hasEntries(mergeExecDiff) ? mergeExecDiff[0] : null; // Pass base as remote, which means fall back to unchanged if no diff: - executionCount = createImmutableModel(execBase, execBase, execDiff); + executionCount = createImmutableModel(parent, execBase, execBase, execDiff); } } } - return new CellDiffModel(source, metadata, outputs, executionCount, base.cell_type); + return new CellDiffModel(parent, source, metadata, outputs, executionCount, base.cell_type); } @@ -126,9 +129,13 @@ function createPatchedCellDecisionDiffModel( */ export class CellMergeModel extends ObjectMergeModel { - constructor(base: nbformat.ICell | null, decisions: MergeDecision[], mimetype: string) { + constructor( + parent: IModel, + base: nbformat.ICell | null, + decisions: MergeDecision[], + mimetype: string) { // TODO: Remove/extend whitelist once we support more - super(base, [], mimetype, ['source', 'metadata', 'outputs', 'execution_count']); + super(parent, base, [], mimetype, ['source', 'metadata', 'outputs', 'execution_count']); this.onesided = false; this._deleteCell = false; this.processDecisions(decisions); @@ -426,7 +433,6 @@ class CellMergeModel extends ObjectMergeModel { this.onesided = true; // We set this to distinguish case 3 from normal if (!hasEntries(md.localDiff)) { // 1. or 2.: - this._local = null; if (!md.remoteDiff || md.remoteDiff.length !== 1) { throw new Error('Merge decision does not conform to expectation: ' + md); } @@ -437,17 +443,18 @@ class CellMergeModel extends ObjectMergeModel { throw new Error('Merge decision does not conform to expectation: ' + md); } let v = first.valuelist[0] as nbformat.ICell; - this._remote = createAddedCellDiffModel(v, this.mimetype); - this._merged = createAddedCellDiffModel(v, this.mimetype); + this._local = null; + this._remote = createAddedCellDiffModel(this, v, this.mimetype); + this._merged = createAddedCellDiffModel(this, v, this.mimetype); } else { // 2. - this._remote = createDeletedCellDiffModel(this.base, this.mimetype); - this._merged = createDeletedCellDiffModel(this.base, this.mimetype); + this._local = createUnchangedCellDiffModel(this, this.base, this.mimetype); + this._remote = createDeletedCellDiffModel(this, this.base, this.mimetype); + this._merged = createUnchangedCellDiffModel(this, this.base, this.mimetype); this.deleteCell = valueIn(md.action, ['remote', 'either']); } } else if (!hasEntries(md.remoteDiff)) { // 1. or 2.: - this._remote = null; if (!md.localDiff || md.localDiff.length !== 1) { throw new Error('Merge decision does not conform to expectation: ' + md); } @@ -458,12 +465,14 @@ class CellMergeModel extends ObjectMergeModel { throw new Error('Merge decision does not conform to expectation: ' + md); } let v = first.valuelist[0] as nbformat.ICell; - this._local = createAddedCellDiffModel(v, this.mimetype); - this._merged = createAddedCellDiffModel(v, this.mimetype); + this._local = createAddedCellDiffModel(this, v, this.mimetype); + this._remote = null; + this._merged = createAddedCellDiffModel(this, v, this.mimetype); } else { // 2. - this._local = createDeletedCellDiffModel(this.base, this.mimetype); - this._merged = createDeletedCellDiffModel(this.base, this.mimetype); + this._local = createDeletedCellDiffModel(this, this.base, this.mimetype); + this._remote = createUnchangedCellDiffModel(this, this.base, this.mimetype); + this._merged = createUnchangedCellDiffModel(this, this.base, this.mimetype); this.deleteCell = valueIn(md.action, ['local', 'either']); } } else { @@ -476,14 +485,14 @@ class CellMergeModel extends ObjectMergeModel { // Identical insertions (this relies on preprocessing to ensure only // one value in valuelist) let v = (md.localDiff[0] as IDiffAddRange).valuelist[0]; - this._local = createAddedCellDiffModel(v, this.mimetype); - this._remote = createAddedCellDiffModel(v, this.mimetype); - this._merged = createAddedCellDiffModel(v, this.mimetype); + this._local = createAddedCellDiffModel(this, v, this.mimetype); + this._remote = createAddedCellDiffModel(this, v, this.mimetype); + this._merged = createAddedCellDiffModel(this, v, this.mimetype); } else { // Identical delections - this._local = createDeletedCellDiffModel(this.base, this.mimetype); - this._remote = createDeletedCellDiffModel(this.base, this.mimetype); - this._merged = createDeletedCellDiffModel(this.base, this.mimetype); + this._local = createDeletedCellDiffModel(this, this.base, this.mimetype); + this._remote = createDeletedCellDiffModel(this, this.base, this.mimetype); + this._merged = createDeletedCellDiffModel(this, this.base, this.mimetype); this.deleteCell = valueIn(md.action, ['local', 'remote', 'either']); } } else { @@ -496,13 +505,13 @@ class CellMergeModel extends ObjectMergeModel { 'cannot have null base for deleted cell: ' + md); } if (ops[0] === 'removerange') { - this._local = createDeletedCellDiffModel(this.base, this.mimetype); + this._local = createDeletedCellDiffModel(this, this.base, this.mimetype); this.deleteCell = md.action === 'local'; // The patch op will be on cell level. Split it on sub keys! newDecisions = newDecisions.concat(this.splitPatch( md, null, md.remoteDiff[0] as IDiffPatchObject)); } else { - this._remote = createDeletedCellDiffModel(this.base, this.mimetype); + this._remote = createDeletedCellDiffModel(this, this.base, this.mimetype); this.deleteCell = md.action === 'remote'; // The patch op will be on cell level. Split it on sub keys! newDecisions = newDecisions.concat(this.splitPatch( @@ -603,9 +612,9 @@ class CellMergeModel extends ObjectMergeModel { throw new Error('Cannot create a patched or unchanged diff model with null base!'); } if (diff && diff.length > 0) { - return createPatchedCellDiffModel(this.base, diff, this.mimetype); + return createPatchedCellDiffModel(this, this.base, diff, this.mimetype); } else { - return createUnchangedCellDiffModel(this.base, this.mimetype); + return createUnchangedCellDiffModel(this, this.base, this.mimetype); } } @@ -614,6 +623,6 @@ class CellMergeModel extends ObjectMergeModel { throw new Error('Cannot create a patched or unchanged merged diff model with null base!'); } return createPatchedCellDecisionDiffModel( - this.base, this.decisions, this.local, this.remote, this.mimetype); + this, this.base, this.decisions, this.local, this.remote, this.mimetype); } } diff --git a/packages/nbdime/src/merge/model/common.ts b/packages/nbdime/src/merge/model/common.ts index 59ac5b6f..e85f16d0 100644 --- a/packages/nbdime/src/merge/model/common.ts +++ b/packages/nbdime/src/merge/model/common.ts @@ -11,7 +11,7 @@ import { } from '../../diff/range'; import { - StringDiffModel, IStringDiffModel + StringDiffModel, IStringDiffModel, IModel } from '../../diff/model'; import { @@ -37,12 +37,12 @@ import { */ export class DecisionStringDiffModel extends StringDiffModel { - constructor(base: any, decisions: MergeDecision[], + constructor(parent: IModel, base: any, decisions: MergeDecision[], sourceModels: (IStringDiffModel | null)[], collapsible?: boolean, header?: string, collapsed?: boolean) { // Set up initial parameters for super call let baseStr = stringifyAndBlankNull(base); - super(baseStr, '', [], [], + super(parent, baseStr, '', [], [], collapsible, header, collapsed); this.rawBase = base; this.decisions = decisions; @@ -144,7 +144,7 @@ class DecisionStringDiffModel extends StringDiffModel { * createMergedDiffModel. */ export -abstract class ObjectMergeModel { +abstract class ObjectMergeModel implements IModel { /** * Create a diff model of the correct type given the diff (which might be @@ -160,9 +160,13 @@ abstract class ObjectMergeModel { - constructor(base: nbformat.INotebookMetadata, decisions: MergeDecision[]) { - super(base, decisions, 'application/json'); + constructor(parent: IModel, base: nbformat.INotebookMetadata, decisions: MergeDecision[]) { + super(parent, base, decisions, 'application/json'); } serialize(): nbformat.INotebookMetadata { @@ -45,15 +45,15 @@ class MetadataMergeModel extends ObjectMergeModel 0) { - return createPatchStringDiffModel(this.base, diff); + return createPatchStringDiffModel(this, this.base, diff); } else { - return createDirectStringDiffModel(this.base, this.base); + return createDirectStringDiffModel(this, this.base, this.base); } } protected createMergedDiffModel(): IStringDiffModel { return new DecisionStringDiffModel( - this.base, this.decisions, + this, this.base, this.decisions, [this.local, this.remote]); } diff --git a/packages/nbdime/src/merge/model/notebook.ts b/packages/nbdime/src/merge/model/notebook.ts index b283c148..bb135881 100644 --- a/packages/nbdime/src/merge/model/notebook.ts +++ b/packages/nbdime/src/merge/model/notebook.ts @@ -28,6 +28,10 @@ import { stringify } from '../../patch'; +import { + IModel +} from '../../diff/model/common'; + import { CellMergeModel } from './cell'; @@ -41,7 +45,7 @@ import { * Diff model for a Jupyter Notebook */ export -class NotebookMergeModel { +class NotebookMergeModel implements IModel { static preprocessDecisions(rawMergeDecisions: IMergeDecision[]): MergeDecision[] { let mergeDecisions: MergeDecision[] = []; @@ -92,7 +96,7 @@ class NotebookMergeModel { this.cells = this.buildCellList(decisions); let metadataDecs = filterDecisions(decisions, ['metadata']); - this.metadata = new MetadataMergeModel(base.metadata, metadataDecs); + this.metadata = new MetadataMergeModel(this, base.metadata, metadataDecs); this.unsavedChanges = false; } @@ -168,6 +172,11 @@ class NotebookMergeModel { */ unsavedChanges: boolean; + /** + * Notebook models never have a parent, so this is always null + */ + parent: IModel | null = null; + /** * Correlate the different cells in the diff lists into a merge list */ @@ -235,7 +244,7 @@ class NotebookMergeModel { let cells: CellMergeModel[] = []; for (let cellInfo of cellDecisions) { cells.push(new CellMergeModel( - cellInfo.base, cellInfo.decisions, this.mimetype)); + this, cellInfo.base, cellInfo.decisions, this.mimetype)); } return cells; diff --git a/packages/nbdime/src/merge/widget/cell.ts b/packages/nbdime/src/merge/widget/cell.ts index aeac7d2a..d86fd0a6 100644 --- a/packages/nbdime/src/merge/widget/cell.ts +++ b/packages/nbdime/src/merge/widget/cell.ts @@ -6,6 +6,10 @@ import { nbformat } from '@jupyterlab/coreutils'; +import { + each +} from '@phosphor/algorithm'; + import { Panel, Widget } from '@phosphor/widgets'; @@ -62,6 +66,7 @@ export const CELLMERGE_CLASS = 'jp-Cell-merge'; const CELL_HEADER_CLASS = 'jp-Merge-cellHeader'; const CELL_HEADER_TITLE_CLASS = 'jp-Merge-cellHeader-title'; +const CELL_HEADER_FOURWAY_CLASS = 'jp-Merge-cellHeader-fourway'; const MARKED_DELETE = 'jp-mod-todelete'; const MARKED_CLEAR_OUTPUTS = 'jp-mod-clearoutputs'; @@ -219,122 +224,141 @@ class CellMergeWidget extends Panel { container.addClass(OUTPUTS_ROW_CLASS); this.addWidget(container); } + + // Create a "force 4-pane view" button + let btn4 = new Widget({ + node: document.createElement('button') + }); + btn4.addClass(CELL_HEADER_FOURWAY_CLASS); + btn4.node.innerText = 'Four-way'; + btn4.node.onclick = event => { + this.forceFourWayViews(); + btn4.parent = null!; + }; + this.header.insertWidget(2, btn4); } else { - // Setup full 4-way mergeview of source, metadata and outputs - // as needed (if changed). Source/metadata/output are each a "row" - let execDec = model.getExecutionCountDecision(); - if (execDec && execDec.action === 'clear') { - let row = new FlexPanel({direction: 'left-to-right'}); - row.addClass(EXECUTIONCOUNT_ROW_CLASS); - let textWidget = new Widget(); - textWidget.node.innerText = 'Execution count will be cleared.'; - row.addWidget(textWidget); - this.addWidget(row); - } - let sourceView: Widget | null = null; - if (model.local && model.local.source.unchanged && - model.remote && model.remote.source.unchanged && - model.merged.source.unchanged) { - // Use single unchanged view of source - sourceView = CellDiffWidget.createView( - model.merged.source, model.merged, CURR_CLASSES, this._rendermime); - } else { - sourceView = CellMergeWidget.createMergeView( - model.local ? model.local.source : null, - model.remote ? model.remote.source : null, - model.merged.source, - CURR_CLASSES); - } - if (sourceView === null) { - throw new Error('Was not able to create merge view for cell!'); - } - this.sourceView = sourceView; - sourceView.addClass(SOURCE_ROW_CLASS); - this.addWidget(sourceView); - - let metadataChanged = false; - let outputsChanged = false; - for (let m of model.subModels) { - if (!m || m.deleted) { - // Don't consider deleted cells - continue; - } - metadataChanged = metadataChanged || ( - !!m.metadata && !m.metadata.unchanged); + this.useFourWayViews(); + } + } - if (m.outputs && m.outputs.length > 0) { - for (let o of m.outputs) { - outputsChanged = outputsChanged || !o.unchanged; - } - } + protected useFourWayViews(force=false) { + let model = this.model; + let CURR_CLASSES = MERGE_CLASSES.slice(); // copy + // Setup full 4-way mergeview of source, metadata and outputs + // as needed (if changed). Source/metadata/output are each a "row" + let execDec = model.getExecutionCountDecision(); + if (execDec && execDec.action === 'clear') { + let row = new FlexPanel({direction: 'left-to-right'}); + row.addClass(EXECUTIONCOUNT_ROW_CLASS); + let textWidget = new Widget(); + textWidget.node.innerText = 'Execution count will be cleared.'; + row.addWidget(textWidget); + this.addWidget(row); + } + let sourceView: Widget | null = null; + if (!force && + model.local && model.local.source.unchanged && + model.remote && model.remote.source.unchanged && + model.merged.source.unchanged) { + // Use single unchanged view of source + sourceView = CellDiffWidget.createView( + model.merged.source, model.merged, CURR_CLASSES, this._rendermime); + } else { + sourceView = CellMergeWidget.createMergeView( + model.local ? model.local.source : null, + model.remote ? model.remote.source : null, + model.merged.source, + CURR_CLASSES); + } + if (sourceView === null) { + throw new Error('Was not able to create merge view for cell!'); + } + this.sourceView = sourceView; + sourceView.addClass(SOURCE_ROW_CLASS); + this.addWidget(sourceView); + + let metadataChanged = false; + let outputsChanged = false; + for (let m of model.subModels) { + if (!m || m.deleted) { + // Don't consider deleted cells + continue; } + metadataChanged = metadataChanged || ( + !!m.metadata && !m.metadata.unchanged); - if (metadataChanged) { - let metadataView = CellMergeWidget.createMergeView( - model.local ? model.local.metadata : null, - model.remote ? model.remote.metadata : null, - model.merged.metadata, - CURR_CLASSES, - true); // Do not allow manual edit of metadata - if (metadataView === null) { - throw new Error('Was not able to create merge view for cell metadata!'); + if (m.outputs && m.outputs.length > 0) { + for (let o of m.outputs) { + outputsChanged = outputsChanged || !o.unchanged; } - this.metadataView = metadataView; - let container = new Panel(); - container.addWidget(metadataView); - - let header = 'Metadata changed'; - let collapser = new CollapsiblePanel(container, header, true); - collapser.addClass(METADATA_ROW_CLASS); - this.addWidget(collapser); } - if (outputsChanged || hasEntries(model.merged.outputs)) { - // We know here that we have code cell - // -> all have outputs !== null - let baseOut = CellMergeWidget.getOutputs( - model.local ? model.local.outputs! : [], true); - let localOut = CellMergeWidget.getOutputs( - model.local ? model.local.outputs! : []); - let remoteOut = CellMergeWidget.getOutputs( - model.remote ? model.remote.outputs! : []); - let mergedOut = CellMergeWidget.getOutputs(model.merged.outputs!); - let view = new RenderableOutputsMergeView( - mergedOut, MERGE_CLASSES, this._rendermime, - baseOut, remoteOut, localOut); - this.outputViews = view; - - let header = outputsChanged ? - (model.outputsConflicted ? - 'Outputs conflicted' : - 'Outputs changed') : - 'Outputs unchanged'; - let collapser = new CollapsiblePanel(view, header, !outputsChanged); - collapser.addClass(OUTPUTS_ROW_CLASS); - - if (model.outputsConflicted) { - collapser.addClass(OUTPUTS_CONFLICTED_CLASS); - let conflictClearBtn = new Widget(); - conflictClearBtn.addClass(MARK_OUTPUTS_RESOLVED_CLASS); - let node = conflictClearBtn.node; - let btn = document.createElement('button'); - btn.onclick = (ev: MouseEvent) => { - if (ev.button !== 0) { - return; // Only main button clicks - } - model.clearOutputConflicts(); - collapser.removeClass(OUTPUTS_CONFLICTED_CLASS); - collapser.headerTitle = 'Outputs changed'; - ev.preventDefault(); - ev.stopPropagation(); - conflictClearBtn.parent = null!; - }; - btn.innerText = 'Mark resolved'; - node.appendChild(btn); - collapser.header.insertWidget(1, conflictClearBtn); - } + } - this.addWidget(collapser); + if (metadataChanged) { + let metadataView = CellMergeWidget.createMergeView( + model.local ? model.local.metadata : null, + model.remote ? model.remote.metadata : null, + model.merged.metadata, + CURR_CLASSES, + true); // Do not allow manual edit of metadata + if (metadataView === null) { + throw new Error('Was not able to create merge view for cell metadata!'); } + this.metadataView = metadataView; + let container = new Panel(); + container.addWidget(metadataView); + + let header = 'Metadata changed'; + let collapser = new CollapsiblePanel(container, header, true); + collapser.addClass(METADATA_ROW_CLASS); + this.addWidget(collapser); + } + if (outputsChanged || hasEntries(model.merged.outputs)) { + // We know here that we have code cell + // -> all have outputs !== null + let baseOut = CellMergeWidget.getOutputs( + model.local ? model.local.outputs! : [], true); + let localOut = CellMergeWidget.getOutputs( + model.local ? model.local.outputs! : []); + let remoteOut = CellMergeWidget.getOutputs( + model.remote ? model.remote.outputs! : []); + let mergedOut = CellMergeWidget.getOutputs(model.merged.outputs!); + let view = new RenderableOutputsMergeView( + mergedOut, MERGE_CLASSES, this._rendermime, + baseOut, remoteOut, localOut); + this.outputViews = view; + + let header = outputsChanged ? + (model.outputsConflicted ? + 'Outputs conflicted' : + 'Outputs changed') : + 'Outputs unchanged'; + let collapser = new CollapsiblePanel(view, header, !outputsChanged); + collapser.addClass(OUTPUTS_ROW_CLASS); + + if (model.outputsConflicted) { + collapser.addClass(OUTPUTS_CONFLICTED_CLASS); + let conflictClearBtn = new Widget(); + conflictClearBtn.addClass(MARK_OUTPUTS_RESOLVED_CLASS); + let node = conflictClearBtn.node; + let btn = document.createElement('button'); + btn.onclick = (ev: MouseEvent) => { + if (ev.button !== 0) { + return; // Only main button clicks + } + model.clearOutputConflicts(); + collapser.removeClass(OUTPUTS_CONFLICTED_CLASS); + collapser.headerTitle = 'Outputs changed'; + ev.preventDefault(); + ev.stopPropagation(); + conflictClearBtn.parent = null!; + }; + btn.innerText = 'Mark resolved'; + node.appendChild(btn); + collapser.header.insertWidget(1, conflictClearBtn); + } + + this.addWidget(collapser); } } @@ -367,6 +391,26 @@ class CellMergeWidget extends Panel { this.header = header; } + /** + * Force a widget to show four-way view + */ + forceFourWayViews(): void { + // Clear previous views: + let layout = this.layout as PanelLayout; + for (let i=layout.widgets.length-1; i >= 0; --i) { + let value = layout.widgets.at(i); + if (value !== this.header) { + value.parent = null!; + } + } + for (let cls of [ + ONEWAY_LOCAL_CLASS, ONEWAY_REMOTE_CLASS, + TWOWAY_ADDITION_CLASS, TWOWAY_DELETION_CLASS]) { + this.removeClass(cls); + } + this.useFourWayViews(true); + } + private _createClearOutputToggle(): Widget { let {checkbox, widget} = createCheckbox( this.model.clearOutputs, 'Clear outputs'); diff --git a/packages/nbdime/src/styles/merge.css b/packages/nbdime/src/styles/merge.css index 966e955b..4767336b 100644 --- a/packages/nbdime/src/styles/merge.css +++ b/packages/nbdime/src/styles/merge.css @@ -107,6 +107,11 @@ background-color: var(--jp-merge-local-color3); } +.jp-Notebook-merge .CodeMirror-merge-m-chunk-custom { background-color: #ffb; } +.jp-Notebook-merge .CodeMirror-merge-m-chunk-start-custom { border-top: 1px solid #fe6; } +.jp-Notebook-merge .CodeMirror-merge-m-chunk-end-custom { border-bottom: 1px solid #fe6; } + + .jp-Notebook-merge .CodeMirror-merge-l-chunk.CodeMirror-merge-r-chunk { background: var(--jp-merge-both-color2); } .jp-Notebook-merge .CodeMirror-merge-l-chunk-start.CodeMirror-merge-r-chunk-start { border-top: 1px solid var(--jp-merge-both-color1); } @@ -129,8 +134,14 @@ } -.jp-Notebook-merge .CodeMirror-merge-m-chunk-start-either { border-top: 1px solid var(--jp-merge-either-color1); } -.jp-Notebook-merge .CodeMirror-merge-m-chunk-end-either { border-bottom: 1px solid var(--jp-merge-either-color1); } +.jp-Notebook-merge .CodeMirror-merge-m-chunk-start-either, +.jp-Notebook-merge .CodeMirror-merge-m-chunk-start-mixed:not(.CodeMirror-merge-m-chunk-custom) { + border-top: 1px solid var(--jp-merge-either-color1); + } +.jp-Notebook-merge .CodeMirror-merge-m-chunk-end-either, +.jp-Notebook-merge .CodeMirror-merge-m-chunk-end-mixed:not(.CodeMirror-merge-m-chunk-custom) { + border-bottom: 1px solid var(--jp-merge-either-color1); + } .jp-Notebook-merge .CodeMirror-merge-m-chunk-end-either-empty + .CodeMirror-linewidget .CodeMirror-merge-spacer { border-bottom: 1px solid var(--jp-merge-either-color1); diff --git a/packages/nbdime/test/src/common/mergeview.spec.ts b/packages/nbdime/test/src/common/mergeview.spec.ts index d1257347..3d837735 100644 --- a/packages/nbdime/test/src/common/mergeview.spec.ts +++ b/packages/nbdime/test/src/common/mergeview.spec.ts @@ -18,7 +18,7 @@ describe('common', () => { it('should be initialized for unchanged diff', () => { let orig = 'Value'; - let remote = createDirectStringDiffModel(orig, orig); + let remote = createDirectStringDiffModel(null!, orig, orig); let p = new MergeView({ orig, remote diff --git a/packages/nbdime/test/src/diff/chunking.spec.ts b/packages/nbdime/test/src/diff/chunking.spec.ts index a86aa37e..4e2b613c 100644 --- a/packages/nbdime/test/src/diff/chunking.spec.ts +++ b/packages/nbdime/test/src/diff/chunking.spec.ts @@ -199,7 +199,7 @@ describe('diff', () => { let remote = 'Single updated line text'; let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw('Single '.length, 'update '.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -214,7 +214,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1\nLine 2 is '.length, 'now '.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -229,7 +229,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1\n'.length, 'Now '.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -244,7 +244,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1\nLine 2 is like this'.length, ' now'.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -259,7 +259,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1'.length, '\nLine 1.1'.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -274,7 +274,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1\n'.length, 'Line 1.1\n'.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -292,7 +292,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1'.length, '\n'.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -310,7 +310,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1'.length, '\n\n\n'.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -328,7 +328,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 'Line 1'.length, '\nLine 1.1\n'.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); @@ -343,7 +343,7 @@ describe('diff', () => { let added: DiffRangeRaw[] = []; added.push(new DiffRangeRaw( 0, 'Line 0\n'.length)); - let m = new StringDiffModel(base, remote, added, []); + let m = new StringDiffModel(null!, base, remote, added, []); let chunks = m.getLineChunks(); expect(chunks).to.have.length(1); expect(chunks[0].baseFrom).to.equal(chunks[0].remoteFrom); diff --git a/packages/nbdime/test/src/diff/model.spec.ts b/packages/nbdime/test/src/diff/model.spec.ts index 65d78458..292f7639 100644 --- a/packages/nbdime/test/src/diff/model.spec.ts +++ b/packages/nbdime/test/src/diff/model.spec.ts @@ -46,7 +46,7 @@ describe('diff', () => { describe('createDirectStringDiffModel', () => { it('should create an added model', () => { - let model = createDirectStringDiffModel(null, 'foobar!'); + let model = createDirectStringDiffModel(null!, null, 'foobar!'); expect(model.added).to.be(true); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(false); @@ -55,7 +55,7 @@ describe('diff', () => { }); it('should create a deleted model', () => { - let model = createDirectStringDiffModel('foobar!', null); + let model = createDirectStringDiffModel(null!, 'foobar!', null); expect(model.added).to.be(false); expect(model.deleted).to.be(true); expect(model.unchanged).to.be(false); @@ -64,7 +64,7 @@ describe('diff', () => { }); it('should create an unchanged model', () => { - let model = createDirectStringDiffModel('foobar!', 'foobar!'); + let model = createDirectStringDiffModel(null!, 'foobar!', 'foobar!'); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(true); @@ -81,7 +81,7 @@ describe('diff', () => { it('should fail for all null inputs', () => { expect(createDirectStringDiffModel).withArgs( - null, null).to.throwException( + null, null, null).to.throwException( /Invalid arguments to createDirectStringDiffModel\(\)/ ); }); @@ -95,7 +95,7 @@ describe('diff', () => { it('should create a patched model', () => { let base = [0, 1, 'foo']; let diff = [opAddRange(2, 'bar')]; - let model = createPatchStringDiffModel(base, diff); + let model = createPatchStringDiffModel(null!, base, diff); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(false); @@ -106,7 +106,7 @@ describe('diff', () => { it('should create an unchanged model by empty diff', () => { let base = 'foobar!'; let diff: IDiffEntry[] = []; - let model = createPatchStringDiffModel(base, diff); + let model = createPatchStringDiffModel(null!, base, diff); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(true); @@ -117,7 +117,7 @@ describe('diff', () => { it('should create an unchanged model by empty diff for non-string input', () => { let base = [0, 1, 'foo']; let diff: IDiffEntry[] = []; - let model = createPatchStringDiffModel(base, diff); + let model = createPatchStringDiffModel(null!, base, diff); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(true); @@ -137,7 +137,7 @@ describe('diff', () => { it('should create an added model', () => { let model = new OutputDiffModel( - null, dummyOutput + null!, null, dummyOutput ); expect(model.added).to.be(true); expect(model.deleted).to.be(false); @@ -146,7 +146,7 @@ describe('diff', () => { it('should create a deleted model', () => { let model = new OutputDiffModel( - dummyOutput, null + null!, dummyOutput, null ); expect(model.added).to.be(false); expect(model.deleted).to.be(true); @@ -155,7 +155,7 @@ describe('diff', () => { it('should create an unchanged model', () => { let model = new OutputDiffModel( - dummyOutput, dummyOutput + null!, dummyOutput, dummyOutput ); expect(model.added).to.be(false); expect(model.deleted).to.be(false); @@ -164,7 +164,7 @@ describe('diff', () => { it('should patch output if given diff and no remote', () => { let diff = [opPatch('text', [opPatch(0, [opAddRange(3, ' bar')])])]; - let model = new OutputDiffModel(dummyOutput, null, diff); + let model = new OutputDiffModel(null!, dummyOutput, null, diff); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(false); @@ -178,7 +178,7 @@ describe('diff', () => { it('should fail for all null input', () => { expect(() => { - new OutputDiffModel(null, null ); + new OutputDiffModel(null!, null, null ); }).to.throwException( /Either remote or base value need to be given/); }); @@ -202,7 +202,7 @@ describe('diff', () => { let mimetype = 'text/python'; it('should be creatable by createAddedCellDiffModel', () => { - let model = createAddedCellDiffModel(codeCellA, mimetype); + let model = createAddedCellDiffModel(null!, codeCellA, mimetype); expect(model.added).to.be(true); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(false); @@ -214,7 +214,7 @@ describe('diff', () => { }); it('should be creatable by createDeletedCellDiffModel', () => { - let model = createDeletedCellDiffModel(codeCellA, mimetype); + let model = createDeletedCellDiffModel(null!, codeCellA, mimetype); expect(model.added).to.be(false); expect(model.deleted).to.be(true); expect(model.unchanged).to.be(false); @@ -226,7 +226,7 @@ describe('diff', () => { }); it('should be creatable by createUnchangedCellDiffModel', () => { - let model = createUnchangedCellDiffModel(codeCellA, mimetype); + let model = createUnchangedCellDiffModel(null!, codeCellA, mimetype); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(true); @@ -240,7 +240,7 @@ describe('diff', () => { describe('createPatchedCellDiffModel', () => { it('should create an unchanged model for null diff', () => { - let model = createPatchedCellDiffModel(codeCellA, null, mimetype); + let model = createPatchedCellDiffModel(null!, codeCellA, null, mimetype); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(true); @@ -252,7 +252,7 @@ describe('diff', () => { }); it('should create an unchanged model for an empty diff', () => { - let model = createPatchedCellDiffModel(codeCellA, [], mimetype); + let model = createPatchedCellDiffModel(null!, codeCellA, [], mimetype); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(true); @@ -267,7 +267,7 @@ describe('diff', () => { let diff = [ opPatch('source', [opAddRange(1, ['l += 2\n'])]) ]; - let model = createPatchedCellDiffModel(codeCellA, diff, mimetype); + let model = createPatchedCellDiffModel(null!, codeCellA, diff, mimetype); expect(model.added).to.be(false); expect(model.deleted).to.be(false); expect(model.unchanged).to.be(false); diff --git a/packages/nbdime/test/src/diff/util.spec.ts b/packages/nbdime/test/src/diff/util.spec.ts index d0e18941..fbae9596 100644 --- a/packages/nbdime/test/src/diff/util.spec.ts +++ b/packages/nbdime/test/src/diff/util.spec.ts @@ -85,17 +85,6 @@ describe('diff', () => { expect(util.stripSource(diff)).to.eql(expected); }); - it('should combine subsequent line additions', () => { - let source = ['test\n', 'foo\n', 'bar\n']; - let sourceDiff = [ - opAddRange(1, ['wee\n']), - opAddRange(1, ['ooh\n']) - ] - let diff = util.flattenStringDiff(source, sourceDiff); - let expected = [opAddRange(source[0].length, 'wee\nooh\n')]; - expect(util.stripSource(diff)).to.eql(expected); - }); - it('should be robust against different line endings', () => { let sourceA = 'test\nfoo\n\nbar\n'; let sourceB = sourceA.replace(/\n/gm, '\r\n'); @@ -112,15 +101,24 @@ describe('diff', () => { (sourceDiffC[1].valuelist as string[])[0] = sourceDiffA[1].valuelist[0].replace(/\n/gm, '\r'); let diff = util.flattenStringDiff(sourceA, sourceDiffA); - let expected = [opAddRange('test\nfoo\n\n'.length, 'wee\nooh\n')]; + let expected = [ + opAddRange('test\nfoo\n\n'.length, 'wee\n'), + opAddRange('test\nfoo\n\n'.length, 'ooh\n') + ]; expect(util.stripSource(diff)).to.eql(expected); diff = util.flattenStringDiff(sourceB, sourceDiffB); - expected = [opAddRange('test\r\nfoo\r\n\r\n'.length, 'wee\r\nooh\r\n')]; + expected = [ + opAddRange('test\r\nfoo\r\n\r\n'.length, 'wee\r\n'), + opAddRange('test\r\nfoo\r\n\r\n'.length, 'ooh\r\n') + ]; expect(util.stripSource(diff)).to.eql(expected); diff = util.flattenStringDiff(sourceC, sourceDiffC); - expected = [opAddRange('test\rfoo\r\r'.length, 'wee\rooh\r')]; + expected = [ + opAddRange('test\rfoo\r\r'.length, 'wee\r'), + opAddRange('test\rfoo\r\r'.length, 'ooh\r') + ]; expect(util.stripSource(diff)).to.eql(expected); }); @@ -134,17 +132,6 @@ describe('diff', () => { expect(util.stripSource(diff)).to.eql(expected); }); - it('should combine subsequent line deletions', () => { - let source = ['test\n', 'foo\n', 'bar\n']; - let sourceDiff = [ - opRemoveRange(1, 1), - opRemoveRange(2, 1) - ]; - let diff = util.flattenStringDiff(source, sourceDiff); - let expected = [opRemoveRange(source[0].length, 'foo\nbar\n'.length)]; - expect(util.stripSource(diff)).to.eql(expected); - }); - }); }); diff --git a/packages/nbdime/test/src/diff/widget/metadata.spec.ts b/packages/nbdime/test/src/diff/widget/metadata.spec.ts index 18da8fd2..3fa87a23 100644 --- a/packages/nbdime/test/src/diff/widget/metadata.spec.ts +++ b/packages/nbdime/test/src/diff/widget/metadata.spec.ts @@ -19,7 +19,7 @@ describe('diff', () => { describe('MetadataDiffWidget', () => { it('should create a widget for an unchanged model', () => { - let model = createDirectStringDiffModel('{}', '{}'); + let model = createDirectStringDiffModel(null!, '{}', '{}'); let widget = new MetadataDiffWidget(model); expect(widget).to.not.be(null); }); diff --git a/packages/nbdime/test/src/merge/model.spec.ts b/packages/nbdime/test/src/merge/model.spec.ts index 665918e4..d72ee868 100644 --- a/packages/nbdime/test/src/merge/model.spec.ts +++ b/packages/nbdime/test/src/merge/model.spec.ts @@ -145,7 +145,7 @@ describe('merge', () => { }; it('should be creatable by base and empty decision set', () => { - let value = new CellMergeModel(codeCellBase, [], mimetype); + let value = new CellMergeModel(null!, codeCellBase, [], mimetype); expect(value.base).to.be(codeCellBase); expect(value.decisions).to.be.empty(); expect(value.mimetype).to.be(mimetype); @@ -153,7 +153,7 @@ describe('merge', () => { describe('cell level decision', () => { let decs: MergeDecision[] = [new MergeDecision(decPatchLvsDelR)]; - let model = new CellMergeModel(codeCellBase, decs, mimetype); + let model = new CellMergeModel(null!, codeCellBase, decs, mimetype); it('should be creatable by base and decision set', () => { expect(model.base).to.be(codeCellBase); @@ -202,7 +202,7 @@ describe('merge', () => { for (let idec of ccDecs) { decs.push(new MergeDecision(idec)); } - let model = new CellMergeModel(codeCellBase, decs, mimetype); + let model = new CellMergeModel(null!, codeCellBase, decs, mimetype); it('should be creatable by base and decision set', () => { expect(model.base).to.be(codeCellBase); diff --git a/packages/nbdime/test/src/merge/widget.spec.ts b/packages/nbdime/test/src/merge/widget.spec.ts index 89d59d55..dcad6927 100644 --- a/packages/nbdime/test/src/merge/widget.spec.ts +++ b/packages/nbdime/test/src/merge/widget.spec.ts @@ -58,7 +58,7 @@ describe('merge', () => { orig_nbformat: 4 } as nbformat.INotebookMetadata; let model = new MetadataMergeModel( - base, []); + null!, base, []); let widget = new MetadataMergeWidget(model); expect(widget).to.not.be(null); }); diff --git a/packages/nbdime/typings/codemirror.d.ts b/packages/nbdime/typings/codemirror.d.ts index 981e6e25..ff75e069 100644 --- a/packages/nbdime/typings/codemirror.d.ts +++ b/packages/nbdime/typings/codemirror.d.ts @@ -186,7 +186,7 @@ declare namespace CodeMirror { /** Sets the gutter marker for the given gutter (identified by its CSS class, see the gutters option) to the given value. Value can be either null, to clear the marker, or a DOM element, to set it. The DOM element will be shown in the specified gutter next to the specified line. */ - setGutterMarker(line: any, gutterID: string, value: HTMLElement | null): CodeMirror.LineHandle; + setGutterMarker(line: CodeMirror.LineHandle | number, gutterID: string, value: HTMLElement | null): CodeMirror.LineHandle; /** Remove all gutter markers in the gutter with the given ID. */ clearGutter(gutterID: string): void; @@ -517,7 +517,7 @@ declare namespace CodeMirror { /** Replace the part of the document between from and to with the given string. from and to must be {line, ch} objects. to can be left off to simply insert the string at position from. */ - replaceRange(replacement: string, from: CodeMirror.Position, to: CodeMirror.Position): void; + replaceRange(replacement: string, from: CodeMirror.Position, to: CodeMirror.Position, origin?: string): void; /** Get the content of line n. */ getLine(n: number): string;