Side-by-side diff viewer breaks lines on word boundaries and makes diffing difficult in some situations
Open, Needs TriagePublic

Description

In this side-by-side diff, a line of code is indented by 69 spaces. Some viewport widths will show this line as aligned with the lines around it, but other widths show this line as misaligned. This is due to phabricator splitting the line at word boundaries rather than at character boundaries.

markwei created this task.Jun 14 2016, 12:32 AM
chad added a subscriber: chad.Jun 14 2016, 12:34 AM

The movie isn't playable for me.

Can you upload a diff that exhibits the bad behavior into one of our test repositories?

I'll do so as soon as I can.

Meanwhile, are you sure you can't play the video file? I can click on Download File for https://secure.phabricator.com/F1686483 and view on both Mac with QuickTime and Ubuntu with the default video player. Maybe it's deserving of a bug report as well?

chad added a comment.Jun 14 2016, 12:48 AM

We still need a complete bug report for this. See Contributing Bug Reports.

I did get the file to download. It think it should have just downloaded on this page though and not sent me to the File page, so I'll follow up on if that's a bug with @epriestley .

avivey added a subscriber: avivey.Jun 14 2016, 8:19 PM

Is this the same issue https://slack-files.com/T029GG40X-F1FA40DUJ-cdf3ae1eec (From IRC a couple of days ago)? Extra-long words, like minified javascript, doesn't get broken.

chad added a comment.Jun 14 2016, 8:41 PM

The diff UI just looks off in the video, each line takes up two lines. I don't know what browser / OS that is nor have any diffs here to reproduce it though. Retina Mac / Chrome maybe?

chad updated the task description. (Show Details)Jun 14 2016, 8:42 PM
chad edited projects, added Bug Report (Needs Information); removed Bug Report.

OK, I didn't actually read the 69-chars indent earlier.
The image on the right looks exactly the way I would expect a 69-deep text diff to look - most of the way to the right, breaking when it needs to.
The one on the left looks like it's not indented much; The double-space is because the lines on the right-side are longer.

I think this will happen if the diff made the following changes:

  1. Indent the entire function by lots of spaces
  2. Make some content change on line 43 (Extra spaces in around animation).

And the view is set to the default "Ignore most spaces" view, which ignores the extra indentation in the lines that weren't modified (40-42).

eadler added a project: Restricted Project.Sep 15 2016, 6:08 PM