Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jest-diff: Add includeChangeCounts and rename Indicator options #8881

Merged
merged 5 commits into from
Aug 29, 2019
Merged

jest-diff: Add includeChangeCounts and rename Indicator options #8881

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fe05610
jest-diff: Add includeChangeCounts option
pedrottimark Aug 27, 2019
95dface
Update CHANGELOG.md
pedrottimark Aug 27, 2019
0761f97
Rename Symbol options as Indicator like git diff
pedrottimark Aug 29, 2019
206dab6
Display indicator following change counts
pedrottimark Aug 29, 2019
364d640
Update the example diff in README.md
pedrottimark Aug 29, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- `[jest-config]` [**BREAKING**] Set default display name color based on runner ([#8689](https://github.com/facebook/jest/pull/8689))
- `[jest-diff]` Add options for colors and symbols ([#8841](https://github.com/facebook/jest/pull/8841))
- `[jest-diff]` [**BREAKING**] Export as ECMAScript module ([#8873](https://github.com/facebook/jest/pull/8873))
- `[jest-diff]` Add `includeChangeCounts` option ([#8881](https://github.com/facebook/jest/pull/8881))
- `[jest-runner]` Warn if a worker had to be force exited ([#8206](https://github.com/facebook/jest/pull/8206))
- `[@jest/test-result]` Create method to create empty `TestResult` ([#8867](https://github.com/facebook/jest/pull/8867))
- `[jest-worker]` [**BREAKING**] Return a promise from `end()`, resolving with the information whether workers exited gracefully ([#8206](https://github.com/facebook/jest/pull/8206))
Expand Down
29 changes: 28 additions & 1 deletion packages/jest-diff/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ diffs[4][1] === 'm'
*/
```

## Advanced import for diffStringsRaw
### Advanced import for diffStringsRaw

Here are all the named imports for the `diffStringsRaw` function:

Expand Down Expand Up @@ -224,6 +224,7 @@ For other applications, you can provide an options object as a third argument:
| `commonSymbol` | `' '` |
| `contextLines` | `5` |
| `expand` | `true` |
| `includeChangeCounts` | `false` |
| `omitAnnotationLines` | `false` |

### Example of options for labels
Expand Down Expand Up @@ -292,6 +293,32 @@ const options = {

A patch mark like `@@ -12,7 +12,9 @@` accounts for omitted common lines.

### Example of option to include change counts

To display the number of change lines at the right of annotation lines:

```js
const a = ['change from', 'common'];
const b = ['change to', 'insert', 'common'];
const options = {
includeChangeCounts: true,
};

const difference = diffLinesUnified(a, b, options);
```

```diff
- Expected 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered the format

-1  Expected
+2  Received

? It is perhaps less likely to be mistaken for the total line counts of Expected/Received if the number is close together with the +/-.
Maybe there's prior art in other diff renderers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You have a gift for identifying realistic first impressions!

Let’s explore alternatives. An earlier prototype was to repeat the + and - something like:

- Expected  -1
+ Received  +2

but the option for alternative symbols like < or > made me think twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is your interpretation if you see a ratio?

- Expected  1 / 2
+ Received  2 / 3

Copy link
Contributor

Choose a reason for hiding this comment

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

What is your interpretation if you see a ratio?

Not very clear to me, esp. since the sign is still far from the number.

Brainstorming things to show both number (not final):

5 Expected (-1)
6 Received (+2)

But I'm not sure if showing the totals is a good idea at all, if you just look at 5,6 it looks like just one line differs when it really could be 11.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree that totals were unclear. Here a line from git diff --stat

1 file changed, 91 insertions(+), 22 deletions(-)

It suggested symbols following counts. For example, with two sets of options:

includeChangeCounts 1a

Let’s keep the labels aligned with the first column of compared content

+ Received 2

Array [
- "change from",
+ "change to",
+ "insert",
"common",
]
```

### Example of option to omit annotation lines

To display only the comparison lines:
Expand Down
149 changes: 111 additions & 38 deletions packages/jest-diff/src/__tests__/__snapshots__/diff.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ exports[`color of text (expanded) 1`] = `
`;

exports[`context number of lines: -1 (5 default) 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<yellow>@@ -6,9 +6,9 @@</>
<dim> 4,</>
Expand All @@ -117,8 +117,8 @@ exports[`context number of lines: -1 (5 default) 1`] = `
`;

exports[`context number of lines: 0 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<yellow>@@ -11,1 +11,0 @@</>
<green>- 9,</>
Expand All @@ -127,8 +127,8 @@ exports[`context number of lines: 0 1`] = `
`;

exports[`context number of lines: 1 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<yellow>@@ -10,4 +10,4 @@</>
<dim> 8,</>
Expand All @@ -139,8 +139,8 @@ exports[`context number of lines: 1 1`] = `
`;

exports[`context number of lines: 2 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<yellow>@@ -9,6 +9,6 @@</>
<dim> 7,</>
Expand All @@ -153,8 +153,8 @@ exports[`context number of lines: 2 1`] = `
`;

exports[`context number of lines: 3.1 (5 default) 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<yellow>@@ -6,9 +6,9 @@</>
<dim> 4,</>
Expand All @@ -170,8 +170,8 @@ exports[`context number of lines: 3.1 (5 default) 1`] = `
`;

exports[`context number of lines: undefined (5 default) 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<yellow>@@ -6,9 +6,9 @@</>
<dim> 4,</>
Expand All @@ -187,36 +187,36 @@ exports[`context number of lines: undefined (5 default) 1`] = `
`;

exports[`diffStringsUnified edge cases empty both a and b 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 0</>
<red>+ Received 0</>

"
`;

exports[`diffStringsUnified edge cases empty only a 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 0</>
<red>+ Received 1</>

<red>+ one-line string</>"
`;

exports[`diffStringsUnified edge cases empty only b 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 0</>

<green>- one-line string</>"
`;

exports[`diffStringsUnified edge cases equal both non-empty 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 0</>
<red>+ Received 0</>

<dim> one-line string</>"
`;

exports[`diffStringsUnified edge cases multiline has no common after clean up chaff 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 2</>
<red>+ Received 2</>

<green>- delete</>
<green>- two</>
Expand All @@ -225,16 +225,16 @@ exports[`diffStringsUnified edge cases multiline has no common after clean up ch
`;

exports[`diffStringsUnified edge cases one-line has no common after clean up chaff 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<green>- delete</>
<red>+ insert</>"
`;

exports[`falls back to not call toJSON if it throws and then objects have differences 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<dim> Object {</>
<green>- \\"line\\": 1,</>
Expand All @@ -247,8 +247,8 @@ exports[`falls back to not call toJSON if serialization has no differences but t
"<dim>Compared values serialize to the same structure.</>
<dim>Printing internal object structure without calling \`toJSON\` instead.</>

<green>- Expected</>
<red>+ Received</>
<green>- Expected 1</>
<red>+ Received 1</>

<dim> Object {</>
<green>- \\"line\\": 1,</>
Expand All @@ -273,33 +273,33 @@ exports[`highlight only the last in odd length of leading spaces (expanded) 1`]
`;

exports[`oneline strings 1`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<green>- ab</>
<red>+ aa</>"
`;

exports[`oneline strings 2`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 1</>

<green>- 123456789</>
<red>+ 234567890</>"
`;

exports[`oneline strings 3`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 1</>
<red>+ Received 2</>

<green>- oneline</>
<red>+ multi</>
<red>+ line</>"
`;

exports[`oneline strings 4`] = `
"<green>- Expected</>
<red>+ Received</>
"<green>- Expected 2</>
<red>+ Received 1</>

<green>- multi</>
<green>- line</>
Expand Down Expand Up @@ -348,7 +348,74 @@ exports[`options common diff 1`] = `
= ]"
`;

exports[`options omitAnnotationLines diff 1`] = `
exports[`options includeChangeCounts false diffLinesUnified 1`] = `
"<green>- Expected</>
<red>+ Received</>

<dim> Array [</>
<green>- \\"delete\\",</>
<green>- \\"change from\\",</>
<red>+ \\"change to\\",</>
<red>+ \\"insert\\",</>
<dim> \\"common\\",</>
<dim> ]</>"
`;

exports[`options includeChangeCounts false diffStringsUnified 1`] = `
"<green>- Expected</>
<red>+ Received</>

<green>- change <inverse>from</></>
<red>+ change <inverse>to</></>
<dim> common</>"
`;

exports[`options includeChangeCounts true padding diffLinesUnified a has 2 digits 1`] = `
"<green>- Before 10</>
<red>+ After 1</>

<dim> common</>
<green>- a</>
<green>- a</>
<green>- a</>
<green>- a</>
<green>- a</>
<green>- a</>
<green>- a</>
<green>- a</>
<green>- a</>
<green>- a</>
<red>+ b</>"
`;

exports[`options includeChangeCounts true padding diffLinesUnified b has 2 digits 1`] = `
"<green>- Before 1</>
<red>+ After 10</>

<dim> common</>
<green>- a</>
<red>+ b</>
<red>+ b</>
<red>+ b</>
<red>+ b</>
<red>+ b</>
<red>+ b</>
<red>+ b</>
<red>+ b</>
<red>+ b</>
<red>+ b</>"
`;

exports[`options includeChangeCounts true padding diffStringsUnified 1`] = `
"<green>- Before 1</>
<red>+ After 1</>

<green>- change <inverse>from</></>
<red>+ change <inverse>to</></>
<dim> common</>"
`;

exports[`options omitAnnotationLines true diff 1`] = `
"<dim> Array [</>
<green>- \\"delete\\",</>
<green>- \\"change from\\",</>
Expand All @@ -358,4 +425,10 @@ exports[`options omitAnnotationLines diff 1`] = `
<dim> ]</>"
`;

exports[`options omitAnnotationLines diffStringsUnified empty strings 1`] = `""`;
exports[`options omitAnnotationLines true diffStringsUnified and includeChangeCounts true 1`] = `
"<green>- change <inverse>from</></>
<red>+ change <inverse>to</></>
<dim> common</>"
`;

exports[`options omitAnnotationLines true diffStringsUnified empty strings 1`] = `""`;
Loading