New issue
Advanced search Search tips

Issue 691883 link

Starred by 3 users
Status: Fixed
Owner:
tkent@chromium.org
Closed: Feb 2017
Cc:
qyears...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression

Blocked on:
issue 672651


Sign in to add a comment

html_diff: Hard to find updated parts.

Project Member Reported by tkent@chromium.org, Feb 14 2017 
What steps will reproduce the problem?
(1) Upload a CL to update a large layout test result
(2) git cl try
(3) Observe a failing layout-test-results page.

e.g.
Open https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_rel_ng/389526/layout-test-results/results.html, and click "pretty diff" for external/wpt/selection/extend-00.html

What is the expected result?
A part with differences should be shown without vertical scrolling.

What happens instead?
We need vertical scroll a lot, and need to find a diff part manually.

Please use labels and text to provide additional information.
Ruby-based PrettyPatch didn't have this issue.  It showed only parts with difference.

Comment 1 by qyears...@chromium.org, Feb 14 2017

Blockedon: 672651
Status: Available (was: Untriaged)
Summary: html_diff: Hard to find updated parts. (was: html_diff: Hard to find updated parts)
You're right - I think in order to make a replacement to fully replace pretty-patch, it would be much better if it could do what pretty-patch did -- which is, instead of taking two strings and making a diff, it took a diff output by git and marked up with HTML.

I was planning to do this before, but then other things got prioritized higher. I would still like to finish making a replacement for pretty-patch ( bug 672651 ).
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e0444a4895f138db12a1722679d8a6be17fad11

commit 0e0444a4895f138db12a1722679d8a6be17fad11
Author: tkent <tkent@chromium.org>
Date: Wed Feb 15 06:19:51 2017

hmtl_diff: Show only three lines before/after a diff block.

Introduces HtmlDiffGenerator class to hold line numbers.

BUG= 691883 

Review-Url: https://codereview.chromium.org/2695993002
Cr-Commit-Position: refs/heads/master@{#450588}

[modify] https://crrev.com/0e0444a4895f138db12a1722679d8a6be17fad11/third_party/WebKit/Tools/Scripts/webkitpy/common/html_diff.py
[modify] https://crrev.com/0e0444a4895f138db12a1722679d8a6be17fad11/third_party/WebKit/Tools/Scripts/webkitpy/common/html_diff_unittest.py

Comment 3 by tkent@chromium.org, Feb 15 2017

Owner: tkent@chromium.org
Status: Fixed (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d69da860fbe6e4b6a0ad3f4a7a79f816a2adfd75

commit d69da860fbe6e4b6a0ad3f4a7a79f816a2adfd75
Author: tkent <tkent@chromium.org>
Date: Sat Feb 18 04:08:37 2017

html_diff: Show context lines at the beginning/end of a file correctly.

If insertion happened at the beginning/end of a file, html_diff
incorrectly skipped to show context lines.  This CL fixes it.

BUG= 691883 

Review-Url: https://codereview.chromium.org/2699083002
Cr-Commit-Position: refs/heads/master@{#451415}

[modify] https://crrev.com/d69da860fbe6e4b6a0ad3f4a7a79f816a2adfd75/third_party/WebKit/Tools/Scripts/webkitpy/common/html_diff.py
[modify] https://crrev.com/d69da860fbe6e4b6a0ad3f4a7a79f816a2adfd75/third_party/WebKit/Tools/Scripts/webkitpy/common/html_diff_unittest.py
►

Sign in to add a comment