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.

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

Collapse big diffs by default #1726

Closed
cpojer opened this issue Sep 17, 2016 · 10 comments
Closed

Collapse big diffs by default #1726

cpojer opened this issue Sep 17, 2016 · 10 comments

Comments

@cpojer
Copy link
Member

cpojer commented Sep 17, 2016

Both for snapshots and regular diffs in other matchers, the diffs often get unwieldy and huge. Let's find a good heuristic to collapse unimportant info by default and add an -e (--expand) flag to print the full diff. It should be intentionally different from --verbose.

Proposed heuristic:

  • Always print small diffs, collapse unimportant diffs over N lines automatically
  • Always print small diffs, collapse unimportant changes based on the ratio between changed/unchanged lines. If there is a high ratio of changed lines, show all changes.
  • ...?
@aaronabramov
Copy link
Contributor

a couple of thoughts:

  • it's important to always print full diffs when running in CI mode.
  • if we implement interactive snapshot updates, we can always show small diffs and then give an option to expand (press 'e' to expand snapshot diffs)

@armandabric
Copy link
Contributor

armandabric commented Sep 17, 2016

if we implement interactive snapshot updates, we can always show small diffs and then give an option to expand (press 'e' to expand snapshot diffs)

👍

If use with in watch mode --watch it could led to an interactive mode close of git stash --patch. It will allow the developer to browse interactively each change and validate each one separately. On each diff the developer could chose between: show next/previous diff, expand, update snapshot, ignore.

@cpojer cpojer modified the milestones: next, 16.0.0 Sep 20, 2016
@thymikee
Copy link
Collaborator

thymikee commented Oct 6, 2016

@cpojer I wonder if something like this would suffice?

screen shot 2016-10-07 at 00 13 51

@cpojer
Copy link
Member Author

cpojer commented Oct 6, 2016

@thymikee yeah that looks great! Why @@? Is that what other diff tools do?

@thymikee
Copy link
Collaborator

thymikee commented Oct 7, 2016

Yup, this is what my git diff tool output looks like, but it shows line number near the change.

Since snapshots are not showing any information about line number, I thought counting the trimmed lines would be just nice to have.

screen shot 2016-10-07 at 08 39 08

@cpojer
Copy link
Member Author

cpojer commented Oct 7, 2016

Yeah I think this is nice! Are you going to send us a PR? :)

@cpojer
Copy link
Member Author

cpojer commented Oct 7, 2016

I think we should keep 4 lines of context at least.

@thymikee
Copy link
Collaborator

thymikee commented Oct 7, 2016

Sure, done!

@cpojer
Copy link
Member Author

cpojer commented Oct 25, 2016

We'll continue the conversation in the PR.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants