feat: add stablePatchmarks option #72
Conversation
src/index.js
Outdated
| @@ -43,6 +44,10 @@ const snapshotDiff = (valueA: any, valueB: any, options?: Options): string => { | |||
| difference = stripAnsi(difference); | |||
| } | |||
|
|
|||
| if (mergedOptions.omitPatchMarks) { | |||
| difference = difference.replace(/^@.*/gm, '-------------') | |||
There was a problem hiding this comment.
I think we can make this regex better and take into account that we have two @@ and numbers with commas between. Also I'm leaning towards replacing them with:
@@ --- --- @@
There was a problem hiding this comment.
Also we only need to check it only if expand === false
There was a problem hiding this comment.
I considered a more rigorous regex, but since jest-diff seems to always pad the start of the line with whitespace, I went the lazy route. But I agree this should be improved. Here's an example of the new regex: https://regex101.com/r/KLDkC0/1
There was a problem hiding this comment.
Ha, left out the 0s in the number range. Updated regex: https://regex101.com/r/KLDkC0/2
README.md
Outdated
| @@ -133,6 +133,8 @@ expect(valueA: any).toMatchDiffSnapshot(valueB: any, options?: Options, testName | |||
| - `expand: boolean` (default: `false`) – expand the diff, so the whole information is preserved | |||
| - `colors: boolean` (default: `false`) – preserve color information from Jest diff | |||
| - `contextLines: number` (default: 5) - number of context lines to be shown at the beginning and at the end of a snapshot | |||
| - `omitPatchMarks: boolean` (default: `false`) - prevent line number patch marks from appearing in | |||
There was a problem hiding this comment.
How about stablePatchmarks? I'm still not sure how to best me it. @SimenB ideas?
There was a problem hiding this comment.
stable is better than omit. Other possible verbs:
normalize
scrub
normalize doesn't communicate a whole lot. scrub communicates what we are doing to the patch mark, while stable communicates the purpose behind it.
There was a problem hiding this comment.
I think I'd still go with stable
README.md
Outdated
| @@ -133,6 +133,8 @@ expect(valueA: any).toMatchDiffSnapshot(valueB: any, options?: Options, testName | |||
| - `expand: boolean` (default: `false`) – expand the diff, so the whole information is preserved | |||
| - `colors: boolean` (default: `false`) – preserve color information from Jest diff | |||
| - `contextLines: number` (default: 5) - number of context lines to be shown at the beginning and at the end of a snapshot | |||
| - `omitPatchMarks: boolean` (default: `false`) - prevent line number patch marks from appearing in | |||
| diffs. This can be helpful when diffs are breaking only because of the patch marks. | |||
There was a problem hiding this comment.
Would also be cool to show an example like: changes @@ -1,1 +1,2 @@ to @@ --- --- @
Solves #69.
I'm identifying patch marks with a simple regex that looks for a line starting with
@. This seems safe sincejest-diffappears to add whitespace at the start of all lines. Therefore, even a line of content that begins with@will end up padded with whitespace in the diff itself.And rather than simply stripping out the patchmarks, I'm replacing them with
-------------. Since the point of this is to avoid broken diffs, this solves that issue while still giving some indication that there are hidden lines in between those parts of the diff. Perhaps there's a better replacement indicator than that. We could certainly preserve the patch mark@@notation like@@ ------------- @@if we think that helps communicate the meaning of the new mark.