[go: up one dir, main page]

Skip to content

Formatter-aware Review Diffs

Release notes

Problem to solve

Merge request diffs often show significant changes that are the result of auto-formatters like Prettier.

It should be possible to detect these kinds of changes that don't introduce meaningful diffs and ignore them (or have an option that ignores these types of diffs manually).

This would help speed up reviews, and reduce the cognitive overhead of mentally diffing lines of code to ensure that the only thing that changed - generally - is whitespace.

The Twitter thread by Jeremy Ashkenas in the Links / References section is the instigating factor for this feature.

Intended users

User experience goal

Proposal

Please see the Twitter thread for some general thoughts on this.

This would almost certainly have to be a User Preference that toggles some feature.
A couple of proposed solutions for this feature with no comments (as they have mixed efficiency, viability):

  1. Apply the project's formatter (e.g. Prettier) settings to removed/modified OLD lines, compare the output to NEW lines, disregard diffs that are identical to the formatter output.
  2. Parse files with an AST to remove visual differences like what formatters add, disregard ALL lines that are functionally identical within the AST.

Further details

This feature is closely related to the existing Show whitespace toggle, but goes farther: lines modified as a result of formatting can be completely ignored separately from whitespace changes for other reasons.
In addition, a formatter (like Prettier) might re-arrange lines or modify certain characters that are otherwise irrelevant.

Permissions and Security

Documentation

Availability & Testing

What does success look like, and how can we measure that?

What is the type of buyer?

Is this a cross-stage feature?

Links / references

Instigating tweet: https://mobile.twitter.com/jashkenas/status/1335980597441744901

Edited by Thomas Randolph