675016 - Have Diff viewer ignore whitespace

Status: RESOLVED INCOMPLETE

(addons.mozilla.org Graveyard :: Admin/Editor Tools, enhancement, P5)

Description

[Andrew Williamson [:eviljeff]]

Sometimes developers will annoyingly add/remove spaces/tabs in their updates.  Standalone diff programs normally have an option to ignore whitespace so it would be good if the AMO one did also.

Comment 1

[Kris Maglione [:kmag]]

https://github.com/jbalogh/zamboni/pull/212

Comment 2

[Andy McKay]

Quickly I think something like:

http://github.com/andymckay/zamboni/commit/73a745

Around line 70, v. small change, will remove whitespace from diffs.

Comment 3

[Wil Clouser [:clouserw]]

Kris: can you review andy's patch in comment 2?

Comment 4

[Kris Maglione [:kmag]]

It appears to work for me in the general case, but I think stripping all white space may be too extreme, as it will ignore merged identifiers and the like. One of the following would be better, I think:

  line.trim().replace(/\s+/g, " ");

or

   line.trim()
       .replace(/\s+/g, " ")
       .replace(/([^a-z0-9_$]) | (?![a-z0-9_$])/gi, "$1");


so that "   foo bar [ baz ] " -> "foo bar[baz]"

Comment 5

[Kris Maglione [:kmag]]

It looks like when lines compare identical, the lines from the right (-) are chosen rather than from the left (+), which leads to some formatting issues. It may be easiest just to switch the sides as we pass them to the diff engine and reverse the indicators we add in prettyHtml.

Comment 6

[Jorge Villalobos [:jorgev] (he/him)]

Who can review this patch?

Comment 7

[Jorge Villalobos [:jorgev] (he/him)]

Resetting a bunch of missed milestones. Sorry for the bugspam.

Comment 8

[Andreas Wagner [:TheOne] [use NI]]

Bad news: All patches disappeared, so we have to start over.
Good news: Kris gave me a rough draft I can use to come up with a patch.

It could take a little while, maybe next week.

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

Kris says he will send you some cookies if you fix this ;)

Comment 10

[Andreas Wagner [:TheOne] [use NI]]

Kris has this been accidentally fixed by your work on the source/diff viewer by any chance?

Comment 11

[Kris Maglione [:kmag]]

Not as far as I know. What makes you think that?

Comment 12

[Andreas Wagner [:TheOne] [use NI]]

Hope/despair...

Comment 13

[Kris Maglione [:kmag]]

Heh. OK. I have other file viewer bugs to fix now anyway.

Comment 14

[Kris Maglione [:kmag]]

https://github.com/mozilla/olympia/pull/694