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

Adjust heuristic for line-based versus byte-based diffing #299

Merged
merged 2 commits into from Sep 2, 2022

Conversation

dsnet
Copy link
Collaborator

@dsnet dsnet commented Jun 29, 2022

If the string has many characters that require escape sequences to print,
then we need to take that into consideration and avoid byte-by-byte diffing.

@dsnet dsnet force-pushed the dsnet/many-escape branch 2 times, most recently from fd1e94b to 35c24e6 Compare June 29, 2022 23:09
@dsnet dsnet requested a review from neild June 29, 2022 23:10
@dsnet
Copy link
Collaborator Author

dsnet commented Jul 13, 2022

Friendly ping @neild

If the string has many characters that require escape sequences to print,
then we need to take that into consideration and avoid byte-by-byte diffing.
... // 3 identical lines
"""
)
>>> TestDiff/Reporter/ManyEscapeCharacters
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This used to print:

  strings.Join({
  	"[\n\t{\"Base32\": \"N",
- 	"A",
+ 	"B",
  	"======\"},\n\t{\"Base32\": \"NBSQ====\"},\n\t{\"Base32\": \"NBSWY===\"},\n\t{\"B",
  	"ase32\": \"NBSWY3A=\"},\n\t{\"Base32\": \"NBSWY3DP\"}\n]",
  }, "")

@dsnet
Copy link
Collaborator Author

dsnet commented Sep 2, 2022

Friendly ping @neild .

We're using cmp to display some diffs of things at Tailscale and this would improve the verbosity of that service.

neild
neild approved these changes Sep 2, 2022
@neild neild merged commit a97318b into master Sep 2, 2022
29 checks passed
@dsnet dsnet deleted the dsnet/many-escape branch September 3, 2022 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants