From d5eb822c673f2d8238abde711d73a52e0d28eefa Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Thu, 7 Aug 2025 09:38:38 -0700 Subject: [PATCH 1/8] ref(issues): improve similar issues stacktrace diff --- static/app/components/splitDiff.tsx | 44 ++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/static/app/components/splitDiff.tsx b/static/app/components/splitDiff.tsx index c063a8830c5e6f..13df9c359be643 100644 --- a/static/app/components/splitDiff.tsx +++ b/static/app/components/splitDiff.tsx @@ -26,20 +26,44 @@ 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}); + + const processResults = ( + currentLine: Change[] = [], + processedLines: Change[][] = [] + ): Change[][] => { + for (const change of results) { + if (change.value.includes('\n')) { + // multiple lines if it should be + const lines = change.value.split('\n'); + for (let i = 0; i < lines.length; i++) { + const lineValue = lines[i]; + if (lineValue !== undefined && lineValue !== '') { + currentLine.push({...change, value: lineValue}); + } + if (i < lines.length - 1) { + processedLines.push(currentLine); + currentLine = []; + } + } + } else { + currentLine.push(change); + } + } + // Push remaining changes if any + if (currentLine.length > 0) { + processedLines.push(currentLine); + } + return processedLines; + }; + + // Call the function and store the result + const groupedChanges = processResults(); return ( - {results.map((line, j) => { + {groupedChanges.map((line, j) => { const highlightAdded = line.find(result => result.added); const highlightRemoved = line.find(result => result.removed); From 7712043640abfdc2f907c2c7b316635ec3b4a7fa Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 11 Aug 2025 10:24:05 -0700 Subject: [PATCH 2/8] add option to choose lines vs words in diff --- static/app/components/issueDiff/index.tsx | 69 ++++++++++++++++------- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/static/app/components/issueDiff/index.tsx b/static/app/components/issueDiff/index.tsx index 6f6e9c78be08e0..8ba1a084067971 100644 --- a/static/app/components/issueDiff/index.tsx +++ b/static/app/components/issueDiff/index.tsx @@ -6,6 +6,7 @@ import type {Location} from 'history'; import {addErrorMessage} from 'sentry/actionCreators/indicator'; import type {Client} from 'sentry/api'; +import {SegmentedControl} from 'sentry/components/core/segmentedControl'; import LoadingIndicator from 'sentry/components/loadingIndicator'; import type SplitDiff from 'sentry/components/splitDiff'; import {t} from 'sentry/locale'; @@ -40,6 +41,7 @@ type Props = { type State = { baseEvent: string[]; + diffType: 'lines' | 'words'; loading: boolean; targetEvent: string[]; SplitDiffAsync?: typeof SplitDiff; @@ -49,13 +51,11 @@ class IssueDiff extends Component { static defaultProps: DefaultProps = defaultProps; state: State = { - loading: true, baseEvent: [], - targetEvent: [], - - // `SplitDiffAsync` is an async-loaded component - // This will eventually contain a reference to the exported component from `./splitDiff` + diffType: 'lines', + loading: true, SplitDiffAsync: undefined, + targetEvent: [], }; componentDidMount() { @@ -160,22 +160,40 @@ class IssueDiff extends Component { render() { const {className} = this.props; - const {SplitDiffAsync: DiffComponent, loading, baseEvent, targetEvent} = this.state; + const { + SplitDiffAsync: DiffComponent, + loading, + baseEvent, + targetEvent, + diffType, + } = this.state; return ( - - {loading && } - {!loading && - DiffComponent && - baseEvent.map((value, i) => ( - - ))} - + + + this.setState({diffType: value as 'lines' | 'words'})} + size="xs" + > + {t('Lines')} + {t('Words')} + + + + {loading && } + {!loading && + DiffComponent && + baseEvent.map((value, i) => ( + + ))} + + ); } } @@ -203,3 +221,16 @@ const StyledIssueDiff = styled('div', { align-items: center; `}; `; + +const SegmentedControlContainer = styled('div')` + position: relative; + display: flex; + justify-content: flex-end; + top: -${space(1)}; + right: ${space(1)}; + z-index: 1; +`; + +const Container = styled('div')` + position: relative; +`; From 9c3416c157dc7aca67b1fd59444b3e0c13a909fd Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 11 Aug 2025 11:02:08 -0700 Subject: [PATCH 3/8] toggle switch --- static/app/components/issueDiff/index.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/static/app/components/issueDiff/index.tsx b/static/app/components/issueDiff/index.tsx index 8ba1a084067971..5e7efb8cf78b55 100644 --- a/static/app/components/issueDiff/index.tsx +++ b/static/app/components/issueDiff/index.tsx @@ -212,6 +212,7 @@ const StyledIssueDiff = styled('div', { flex: 1; display: flex; flex-direction: column; + min-height: 0; ${p => p.loading && @@ -226,11 +227,15 @@ const SegmentedControlContainer = styled('div')` position: relative; display: flex; justify-content: flex-end; + align-items: center; top: -${space(1)}; - right: ${space(1)}; + right: 0; z-index: 1; `; const Container = styled('div')` position: relative; + height: 100%; + display: flex; + flex-direction: column; `; From a6e7ee1f6af74d277cdc2a63718b341ce97e4882 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 11 Aug 2025 13:05:54 -0700 Subject: [PATCH 4/8] back to line --- static/app/components/issueDiff/index.tsx | 74 ++++++----------------- static/app/components/splitDiff.tsx | 29 +++++---- 2 files changed, 33 insertions(+), 70 deletions(-) diff --git a/static/app/components/issueDiff/index.tsx b/static/app/components/issueDiff/index.tsx index 5e7efb8cf78b55..c389971bc2b370 100644 --- a/static/app/components/issueDiff/index.tsx +++ b/static/app/components/issueDiff/index.tsx @@ -6,7 +6,6 @@ import type {Location} from 'history'; import {addErrorMessage} from 'sentry/actionCreators/indicator'; import type {Client} from 'sentry/api'; -import {SegmentedControl} from 'sentry/components/core/segmentedControl'; import LoadingIndicator from 'sentry/components/loadingIndicator'; import type SplitDiff from 'sentry/components/splitDiff'; import {t} from 'sentry/locale'; @@ -41,7 +40,6 @@ type Props = { type State = { baseEvent: string[]; - diffType: 'lines' | 'words'; loading: boolean; targetEvent: string[]; SplitDiffAsync?: typeof SplitDiff; @@ -51,11 +49,13 @@ class IssueDiff extends Component { static defaultProps: DefaultProps = defaultProps; state: State = { - baseEvent: [], - diffType: 'lines', loading: true, - SplitDiffAsync: undefined, + baseEvent: [], targetEvent: [], + + // `SplitDiffAsync` is an async-loaded component + // This will eventually contain a reference to the exported component from `./splitDiff` + SplitDiffAsync: undefined, }; componentDidMount() { @@ -160,40 +160,22 @@ class IssueDiff extends Component { render() { const {className} = this.props; - const { - SplitDiffAsync: DiffComponent, - loading, - baseEvent, - targetEvent, - diffType, - } = this.state; + const {SplitDiffAsync: DiffComponent, loading, baseEvent, targetEvent} = this.state; return ( - - - this.setState({diffType: value as 'lines' | 'words'})} - size="xs" - > - {t('Lines')} - {t('Words')} - - - - {loading && } - {!loading && - DiffComponent && - baseEvent.map((value, i) => ( - - ))} - - + + {loading && } + {!loading && + DiffComponent && + baseEvent.map((value, i) => ( + + ))} + ); } } @@ -212,7 +194,6 @@ const StyledIssueDiff = styled('div', { flex: 1; display: flex; flex-direction: column; - min-height: 0; ${p => p.loading && @@ -222,20 +203,3 @@ const StyledIssueDiff = styled('div', { align-items: center; `}; `; - -const SegmentedControlContainer = styled('div')` - position: relative; - display: flex; - justify-content: flex-end; - align-items: center; - top: -${space(1)}; - right: 0; - z-index: 1; -`; - -const Container = styled('div')` - position: relative; - height: 100%; - display: flex; - flex-direction: column; -`; diff --git a/static/app/components/splitDiff.tsx b/static/app/components/splitDiff.tsx index 13df9c359be643..93a100efe04720 100644 --- a/static/app/components/splitDiff.tsx +++ b/static/app/components/splitDiff.tsx @@ -28,26 +28,26 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { const results = diffFn(base, target, {newlineIsToken: true}); + // split one change that includes multiple linse into one change per line (for formatting) const processResults = ( currentLine: Change[] = [], processedLines: Change[][] = [] ): Change[][] => { for (const change of results) { - if (change.value.includes('\n')) { - // multiple lines if it should be - const lines = change.value.split('\n'); - for (let i = 0; i < lines.length; i++) { - const lineValue = lines[i]; - if (lineValue !== undefined && lineValue !== '') { - currentLine.push({...change, value: lineValue}); - } - if (i < lines.length - 1) { - processedLines.push(currentLine); - currentLine = []; - } + 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 = []; } - } else { - currentLine.push(change); } } // Push remaining changes if any @@ -57,7 +57,6 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { return processedLines; }; - // Call the function and store the result const groupedChanges = processResults(); return ( From 8310f118136fe031bca636c2b542f77421e33193 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Mon, 11 Aug 2025 15:44:58 -0700 Subject: [PATCH 5/8] ref(grouping): fix similar issues diff --- .../replays/diff/replayTextDiff.tsx | 2 +- static/app/components/splitDiff.tsx | 36 ++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/static/app/components/replays/diff/replayTextDiff.tsx b/static/app/components/replays/diff/replayTextDiff.tsx index c7ee91c3ac11dc..0d9a6a1ac713e5 100644 --- a/static/app/components/replays/diff/replayTextDiff.tsx +++ b/static/app/components/replays/diff/replayTextDiff.tsx @@ -52,7 +52,7 @@ export function ReplayTextDiff() { - + ); diff --git a/static/app/components/splitDiff.tsx b/static/app/components/splitDiff.tsx index 93a100efe04720..d31f2a7e942735 100644 --- a/static/app/components/splitDiff.tsx +++ b/static/app/components/splitDiff.tsx @@ -28,11 +28,10 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { const results = diffFn(base, target, {newlineIsToken: true}); - // split one change that includes multiple linse into one change per line (for formatting) - const processResults = ( - currentLine: Change[] = [], - processedLines: Change[][] = [] - ): Change[][] => { + // 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++) { @@ -50,7 +49,6 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { } } } - // Push remaining changes if any if (currentLine.length > 0) { processedLines.push(currentLine); } @@ -66,11 +64,33 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { 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); + })() + : line; + return ( - {line + {displayData .filter(result => !result.added) .map((result, i) => ( @@ -84,7 +104,7 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { - {line + {displayData .filter(result => !result.removed) .map((result, i) => ( From 0d9bd0fe79642debc3bb3afb63cb28024b218752 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Thu, 14 Aug 2025 12:40:36 -0700 Subject: [PATCH 6/8] don't use iife --- static/app/components/splitDiff.tsx | 55 ++++++++++++++++------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/static/app/components/splitDiff.tsx b/static/app/components/splitDiff.tsx index d31f2a7e942735..0c623fc477d616 100644 --- a/static/app/components/splitDiff.tsx +++ b/static/app/components/splitDiff.tsx @@ -23,6 +23,35 @@ type Props = { type?: keyof typeof diffFnMap; }; +// this function splits the lines from diffLines into words that are diffed +function getDisplayData( + line: Change[], + highlightAdded: Change | undefined, + highlightRemoved: Change | undefined +): Change[] { + 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), + '' + ); + + if (!leftText && !rightText) { + return line; + } + + return diffWords(leftText, rightText); + })() + : line; + + return displayData; +} + function SplitDiff({className, type = 'lines', base, target}: Props) { const diffFn = diffFnMap[type]; @@ -64,33 +93,11 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { 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); - })() - : line; - return ( - {displayData + {getDisplayData(line, highlightAdded, highlightRemoved) .filter(result => !result.added) .map((result, i) => ( @@ -104,7 +111,7 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { - {displayData + {getDisplayData(line, highlightAdded, highlightRemoved) .filter(result => !result.removed) .map((result, i) => ( From 838caf79d4608b577f7dbbc09e8782f23394335b Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Thu, 14 Aug 2025 12:48:52 -0700 Subject: [PATCH 7/8] actually remove iife --- static/app/components/splitDiff.tsx | 33 +++++++++++++---------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/static/app/components/splitDiff.tsx b/static/app/components/splitDiff.tsx index 0c623fc477d616..0cf3abfefa5521 100644 --- a/static/app/components/splitDiff.tsx +++ b/static/app/components/splitDiff.tsx @@ -29,27 +29,24 @@ function getDisplayData( highlightAdded: Change | undefined, highlightRemoved: Change | undefined ): Change[] { - 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), - '' - ); + if (!highlightAdded && !highlightRemoved) { + return line; + } - if (!leftText && !rightText) { - return line; - } + const leftText = line.reduce( + (acc, result) => (result.added ? acc : acc + result.value), + '' + ); + const rightText = line.reduce( + (acc, result) => (result.removed ? acc : acc + result.value), + '' + ); - return diffWords(leftText, rightText); - })() - : line; + if (!leftText && !rightText) { + return line; + } - return displayData; + return diffWords(leftText, rightText); } function SplitDiff({className, type = 'lines', base, target}: Props) { From b6d10ba9a38b7ad56f5cdc961d37fc03b19d9cf2 Mon Sep 17 00:00:00 2001 From: Shayna Chambless Date: Thu, 14 Aug 2025 14:18:54 -0700 Subject: [PATCH 8/8] wrap groupedChanges in useMemo --- static/app/components/splitDiff.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/static/app/components/splitDiff.tsx b/static/app/components/splitDiff.tsx index 0cf3abfefa5521..e3bbce5458650f 100644 --- a/static/app/components/splitDiff.tsx +++ b/static/app/components/splitDiff.tsx @@ -1,3 +1,4 @@ +import {useMemo} from 'react'; import styled from '@emotion/styled'; import type {Change} from 'diff'; import {diffChars, diffLines, diffWords} from 'diff'; @@ -55,7 +56,7 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { const results = diffFn(base, target, {newlineIsToken: true}); // split one change that includes multiple lines into one change per line (for formatting) - const processResults = (): Change[][] => { + const groupedChanges = useMemo((): Change[][] => { let currentLine: Change[] = []; const processedLines: Change[][] = []; for (const change of results) { @@ -79,9 +80,7 @@ function SplitDiff({className, type = 'lines', base, target}: Props) { processedLines.push(currentLine); } return processedLines; - }; - - const groupedChanges = processResults(); + }, [results]); return (