Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion static/app/components/issueDiff/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class IssueDiff extends Component<Props, State> {
key={i}
base={value}
target={targetEvent[i] ?? ''}
type="words"
type="lines"
/>
))}
</StyledIssueDiff>
Expand Down
2 changes: 1 addition & 1 deletion static/app/components/replays/diff/replayTextDiff.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function ReplayTextDiff() {
</After>
</ContentSliderDiff.Header>
<SplitDiffScrollWrapper>
<SplitDiff base={leftBody ?? ''} target={rightBody ?? ''} type="words" />
<SplitDiff base={leftBody ?? ''} target={rightBody ?? ''} type="lines" />
</SplitDiffScrollWrapper>
</Container>
);
Expand Down
67 changes: 55 additions & 12 deletions static/app/components/splitDiff.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,71 @@ type Props = {
function SplitDiff({className, type = 'lines', base, target}: Props) {
const diffFn = diffFnMap[type];

const baseLines = base.split('\n');
const targetLines = target.split('\n');
const [largerArray] =
baseLines.length > targetLines.length
? [baseLines, targetLines]
: [targetLines, baseLines];
const results = largerArray.map((_line, index) =>
diffFn(baseLines[index] || '', targetLines[index] || '', {newlineIsToken: true})
);
const results = diffFn(base, target, {newlineIsToken: true});

// split one change that includes multiple lines into one change per line (for formatting)
const processResults = (): Change[][] => {
let currentLine: Change[] = [];
const processedLines: Change[][] = [];
for (const change of results) {
const lines = change.value.split('\n');
for (let i = 0; i < lines.length; i++) {
const lineValue = lines[i];
if (lineValue !== undefined && lineValue !== '') {
currentLine.push({
value: lineValue,
added: change.added,
removed: change.removed,
});
}
if (i < lines.length - 1) {
processedLines.push(currentLine);
currentLine = [];
}
}
}
if (currentLine.length > 0) {
processedLines.push(currentLine);
}
return processedLines;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Diff Visualization Errors in SplitDiff

The processResults function in SplitDiff has two flaws: it incorrectly separates removed and added lines that are part of a replacement, preventing side-by-side diff visualization; and its lineValue !== '' check filters out empty lines, hiding changes involving blank lines and altering content structure in diffs.

Fix in Cursor Fix in Web

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets wrap this in useMemo


const groupedChanges = processResults();

return (
<SplitTable className={className} data-test-id="split-diff">
<SplitBody>
{results.map((line, j) => {
{groupedChanges.map((line, j) => {
const highlightAdded = line.find(result => result.added);
const highlightRemoved = line.find(result => result.removed);

// Apply word-level diffing only to lines that have changes
const displayData =
highlightAdded || highlightRemoved
? (() => {
const leftText = line.reduce(
(acc, result) => (result.added ? acc : acc + result.value),
''
);
const rightText = line.reduce(
(acc, result) => (result.removed ? acc : acc + result.value),
''
);

// Handle edge cases where text might be empty
if (!leftText && !rightText) {
return line; // Return original line if both texts are empty
}

return diffWords(leftText, rightText);
})()
Copy link
Member

@scttcper scttcper Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use iife, make a small named function somewhere

: line;

return (
<tr key={j}>
<Cell isRemoved={highlightRemoved}>
<Line>
{line
{displayData
.filter(result => !result.added)
.map((result, i) => (
<Word key={i} isRemoved={result.removed}>
Expand All @@ -61,7 +104,7 @@ function SplitDiff({className, type = 'lines', base, target}: Props) {

<Cell isAdded={highlightAdded}>
<Line>
{line
{displayData
.filter(result => !result.removed)
.map((result, i) => (
<Word key={i} isAdded={result.added}>
Expand Down
Loading