Skip to content

Commit

Permalink
absorb: switch to immutable data structures
Browse files Browse the repository at this point in the history
Summary:
Similar to converting linelog to use immutable data structures, the absorb states
would be nicer to work with when they are immutable.
So let's convert the types to immutable.

Reviewed By: muirdm

Differential Revision: D67060052

fbshipit-source-id: 54f110704837f2f69947abb35ea041c362fcb1a9
  • Loading branch information
quark-zju authored and facebook-github-bot committed Dec 12, 2024
1 parent a766170 commit d84f45f
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 39 deletions.
6 changes: 3 additions & 3 deletions addons/isl/src/stackEdit/__tests__/absorb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ describe('analyseFileStack', () => {
const chunks = analyseFileStack(stack, injectNewLines('abc'));
expect(applyChunks(stack, chunks)).toMatchInlineSnapshot(`" a ab abc"`);
// Tweak the `selectedRev` so the 1->a, 2->b changes happen at the last rev.
const chunks2 = chunks.map(c => ({...c, selectedRev: 3}));
const chunks2 = chunks.map(c => c.set('selectedRev', 3));
expect(applyChunks(stack, chunks2)).toMatchInlineSnapshot(`" 1 12 abc"`);
// Drop the "2->b" change by setting selectedRev to `null`.
const chunks3 = chunks.map(c => (c.oldStart === 1 ? {...c, selectedRev: null} : c));
const chunks3 = chunks.map(c => (c.oldStart === 1 ? c.set('selectedRev', null) : c));
expect(applyChunks(stack, chunks3)).toMatchInlineSnapshot(`" a a2 a2c"`);
});

Expand Down Expand Up @@ -164,7 +164,7 @@ describe('analyseFileStack', () => {
.join('\n');
}

function applyChunks(stack: FileStackState, chunks: AbsorbDiffChunk[]): string {
function applyChunks(stack: FileStackState, chunks: Iterable<AbsorbDiffChunk>): string {
return compactTexts(applyFileStackEdits(stack, chunks).convertToPlainText());
}

Expand Down
94 changes: 58 additions & 36 deletions addons/isl/src/stackEdit/absorb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,22 @@

import type {LineInfo} from '../linelog';
import type {Rev} from './fileStackState';
import type {RecordOf} from 'immutable';

import {assert} from '../utils';
import {FileStackState} from './fileStackState';
import {List, Record} from 'immutable';
import {diffLines, splitLines} from 'shared/diff';
import {dedup, nullthrows} from 'shared/utils';

/** A diff chunk analyzed by `analyseFileStack`. */
export type AbsorbDiffChunk = {
export type AbsorbDiffChunkProps = {
/** The start line of the old content (start from 0, inclusive). */
oldStart: number;
/** The end line of the old content (start from 0, exclusive). */
oldEnd: number;
/** The new content to replace the old content. */
newLines: string[];
newLines: List<string>;
/**
* Which rev introduces the "old" chunk.
* The following revs are expected to contain this chunk too.
Expand All @@ -37,12 +39,21 @@ export type AbsorbDiffChunk = {
selectedRev: Rev | null;
};

export const AbsorbDiffChunk = Record<AbsorbDiffChunkProps>({
oldStart: 0,
oldEnd: 0,
newLines: List(),
introductionRev: 0,
selectedRev: null,
});
export type AbsorbDiffChunk = RecordOf<AbsorbDiffChunkProps>;

/**
* Given a stack and the latest changes (usually at the stack top),
* calculate the diff chunks and the revs that they might be absorbed to.
* The rev 0 of the file stack should come from a "public" (immutable) commit.
*/
export function analyseFileStack(stack: FileStackState, newText: string): Array<AbsorbDiffChunk> {
export function analyseFileStack(stack: FileStackState, newText: string): List<AbsorbDiffChunk> {
assert(stack.revLength > 0, 'stack should not be empty');
const linelog = stack.convertToLineLog();
const oldRev = stack.revLength - 1;
Expand All @@ -69,24 +80,28 @@ export function analyseFileStack(stack: FileStackState, newText: string): Array<
// Only one rev. Set selectedRev to this.
// For simplicity, we're not checking the "continuous" lines here yet (different from Python).
const introductionRev = involvedRevs[0];
result.push({
oldStart: a1,
oldEnd: a2,
newLines: newLines.slice(b1, b2),
introductionRev,
selectedRev: introductionRev,
});
result.push(
AbsorbDiffChunk({
oldStart: a1,
oldEnd: a2,
newLines: List(newLines.slice(b1, b2)),
introductionRev,
selectedRev: introductionRev,
}),
);
} else if (b1 === b2) {
// Deletion. Break the chunk into sub-chunks with different selectedRevs.
// For simplicity, we're not checking the "continuous" lines here yet (different from Python).
splitChunk(a1, a2, oldLineInfos, (oldStart, oldEnd, introductionRev) => {
result.push({
oldStart,
oldEnd,
newLines: [],
introductionRev,
selectedRev: introductionRev,
});
result.push(
AbsorbDiffChunk({
oldStart,
oldEnd,
newLines: List([]),
introductionRev,
selectedRev: introductionRev,
}),
);
});
} else if (a2 - a1 === b2 - b1 && involvedLineInfos.every(info => info.rev > 0)) {
// Line count matches on both side. No public lines.
Expand All @@ -95,13 +110,15 @@ export function analyseFileStack(stack: FileStackState, newText: string): Array<
// still break the chunks to individual lines.
const delta = b1 - a1;
splitChunk(a1, a2, oldLineInfos, (oldStart, oldEnd, introductionRev) => {
result.push({
oldStart,
oldEnd,
newLines: newLines.slice(oldStart + delta, oldEnd + delta),
introductionRev,
selectedRev: introductionRev,
});
result.push(
AbsorbDiffChunk({
oldStart,
oldEnd,
newLines: List(newLines.slice(oldStart + delta, oldEnd + delta)),
introductionRev,
selectedRev: introductionRev,
}),
);
});
} else {
// Other cases, like replacing 10 lines from 3 revs to 20 lines.
Expand All @@ -111,16 +128,18 @@ export function analyseFileStack(stack: FileStackState, newText: string): Array<
// For now, we just report this chunk as a whole chunk that can
// only be absorbed to the "max" rev where the left side is
// "settled" down.
result.push({
oldStart: a1,
oldEnd: a2,
newLines: newLines.slice(b1, b2),
introductionRev: Math.max(0, ...involvedRevs),
selectedRev: null,
});
result.push(
AbsorbDiffChunk({
oldStart: a1,
oldEnd: a2,
newLines: List(newLines.slice(b1, b2)),
introductionRev: Math.max(0, ...involvedRevs),
selectedRev: null,
}),
);
}
});
return result;
return List(result);
}

/**
Expand All @@ -129,7 +148,7 @@ export function analyseFileStack(stack: FileStackState, newText: string): Array<
*/
export function applyFileStackEdits(
stack: FileStackState,
chunks: readonly AbsorbDiffChunk[],
chunks: Iterable<AbsorbDiffChunk>,
): FileStackState {
// See also [apply](https://github.com/facebook/sapling/blob/6f29531e83daa62d9bd3bc58b712755d34f41493/eden/scm/sapling/ext/absorb/__init__.py#L321)
assert(stack.revLength > 0, 'stack should not be empty');
Expand All @@ -139,12 +158,15 @@ export function applyFileStackEdits(
const oldRev = stack.revLength - 1;
// Apply the changes. Assuming there are no overlapping chunks, we apply
// from end to start so the line numbers won't need change.
const sortedChunks = chunks
const sortedChunks = [...chunks]
.filter(c => c.selectedRev != null)
.toSorted((a, b) => b.oldEnd - a.oldEnd);
sortedChunks.forEach(chunk => {
const targetRev = nullthrows(chunk.selectedRev);
assert(targetRev >= chunk.introductionRev, 'selectedRev must be >= introductionRev');
assert(
targetRev >= chunk.introductionRev,
`selectedRev ${targetRev} must be >= introductionRev ${chunk.introductionRev}`,
);
assert(
targetRev > 0,
'selectedRev must be > 0 since rev 0 is from the immutable public commit',
Expand All @@ -156,7 +178,7 @@ export function applyFileStackEdits(
chunk.oldStart,
chunk.oldEnd,
targetRev * 2 + 1,
chunk.newLines,
chunk.newLines.toArray(),
);
});
const texts = Array.from({length: stack.revLength}, (_, i) => linelog.checkOut(i * 2 + 1));
Expand Down

0 comments on commit d84f45f

Please sign in to comment.