Have Diff viewer ignore whitespace

NEW
Unassigned

Status

addons.mozilla.org Graveyard
Admin/Editor Tools
P5
enhancement
7 years ago
2 years ago

People

(Reporter: eviljeff, Unassigned)

Tracking

Details

(Whiteboard: [ReviewTeam:P2])

(Reporter)

Description

7 years ago
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.
Assignee: nobody → mbasta
Severity: normal → enhancement
Priority: -- → P3
Whiteboard: [required amo-editors]
Target Milestone: --- → Q3 2011
Assignee: mbasta → nobody
Priority: P3 → P5
Target Milestone: Q3 2011 → Future
https://github.com/jbalogh/zamboni/pull/212
Assignee: nobody → maglione.k
Whiteboard: [required amo-editors] → [required amo-editors][patch][needs review]
Target Milestone: Future → 6.1.9

Comment 2

7 years ago
Quickly I think something like:

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

Around line 70, v. small change, will remove whitespace from diffs.
Target Milestone: 6.1.9 → Q3 2011
Kris: can you review andy's patch in comment 2?
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]"
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.
Who can review this patch?
Whiteboard: [required amo-editors][patch][needs review] → [patch][needs review][ReviewTeam]
Resetting a bunch of missed milestones. Sorry for the bugspam.
Target Milestone: Q3 2011 → ---
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.
Assignee: kmaglione+bmo → mail
Whiteboard: [patch][needs review][ReviewTeam] → [ReviewTeam]
Kris says he will send you some cookies if you fix this ;)

Updated

4 years ago
Whiteboard: [ReviewTeam] → [ReviewTeam:P2]
Kris has this been accidentally fixed by your work on the source/diff viewer by any chance?
Flags: needinfo?(kmaglione+bmo)
Not as far as I know. What makes you think that?
Flags: needinfo?(kmaglione+bmo)
Hope/despair...
Heh. OK. I have other file viewer bugs to fix now anyway.
Assignee: awagner → kmaglione+bmo
(Assignee)

Updated

2 years ago
Product: addons.mozilla.org → addons.mozilla.org Graveyard

Updated

2 years ago
Assignee: kmaglione+bmo → nobody
You need to log in before you can comment on or make changes to this bug.