Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 358159 - Option to ignore whitespace differences in compare viewer
Summary: Option to ignore whitespace differences in compare viewer
Status: RESOLVED FIXED
Alias: None
Product: Orion
Classification: ECD
Component: Client (show other bugs)
Version: 0.3   Edit
Hardware: PC All
: P3 normal with 1 vote (vote)
Target Milestone: 7.0   Edit
Assignee: libing wang CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 415591
  Show dependency tree
 
Reported: 2011-09-19 16:00 EDT by Mark Macdonald CLA
Modified: 2015-01-29 10:05 EST (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Macdonald CLA 2011-09-19 16:00:32 EDT
Sometimes it is helpful to hide differences that consist of whitespace when comparing 2 revisions. This would be similar to the --ignore-space-change flag supported by Git diff.

For an example of where this is useful, see commit 18abcc4e in the client repo [1].


[1] http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=18abcc4ef845e412140821195755da3748d90564
Comment 1 Felipe Heidrich CLA 2011-10-27 16:22:27 EDT
+1

Please consider this feature for the next release
Comment 2 Tomasz Zarna CLA 2011-10-28 07:01:22 EDT
That would be an easy fix on the server for git diffs, but I guess you would like to have a server/SCM-agnostic solution, implemented on the client side.
Comment 3 libing wang CLA 2011-10-28 09:26:54 EDT
(In reply to comment #2)
> That would be an easy fix on the server for git diffs, but I guess you would
> like to have a server/SCM-agnostic solution, implemented on the client side.

If client side is to filter this case out,  I would say we should put options on UI. 
By the way, Simon also mentioned before about comparing any two files (e.g.: a file from WebDev VS Orion). I put a bullet on the .4 plan, lets see how the plan is finalized and how we can support this.
Comment 4 Tomasz Zarna CLA 2011-10-28 10:30:52 EDT
(In reply to comment #3)
> Simon also mentioned before about comparing any two files

I could put up a diff service on the server that would accept any two files (strings) and spit out a diff. Would that work for you?
Comment 5 Evan Hughes CLA 2011-11-25 10:46:18 EST
For the Jazz case we'd rather rerun the diff on the server side.
Comment 6 Silenio Quarti CLA 2013-06-21 12:57:03 EDT
I really miss this feature.
Comment 7 Adrian Aichner CLA 2013-08-20 13:55:59 EDT
Looks like jsdiff is being used which already support three distinct diff modes:

diff.diffWords
diff.diffCss
diff.diffLines

I would also like to see support for ignore letter case, but have not found any support for that in jsdiff.

Is somebody already looking into this feature request?
Comment 8 Adrian Aichner CLA 2013-08-20 13:57:36 EDT
I should have mentioned that diffWords should give as the feature asked for in this feature request.
Comment 9 libing wang CLA 2013-08-21 09:39:51 EDT
(In reply to comment #7)
> Looks like jsdiff is being used which already support three distinct diff
> modes:
> 
> diff.diffWords
> diff.diffCss
> diff.diffLines
> 
> I would also like to see support for ignore letter case, but have not found
> any support for that in jsdiff.
> 
> Is somebody already looking into this feature request?
Please open a separate bug.
Comment 10 Adrian Aichner CLA 2013-08-21 10:13:59 EDT
(In reply to comment #9)
> (In reply to comment #7)
> > Looks like jsdiff is being used which already support three distinct diff
> > modes:
> > 
> > diff.diffWords
> > diff.diffCss
> > diff.diffLines
> > 
> > I would also like to see support for ignore letter case, but have not found
> > any support for that in jsdiff.
> > 
> > Is somebody already looking into this feature request?
> Please open a separate bug.

I just submitted Bug 415591.

I should have this clearer on "this feature" to mean ignore-space.

Will ignore whitespace differences in compare viewer be supported soon?
Comment 11 libing wang CLA 2013-08-21 14:28:06 EDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #7)
> > > Looks like jsdiff is being used which already support three distinct diff
> > > modes:
> > > 
> > > diff.diffWords
> > > diff.diffCss
> > > diff.diffLines
> > > 
> > > I would also like to see support for ignore letter case, but have not found
> > > any support for that in jsdiff.
> > > 
> > > Is somebody already looking into this feature request?
> > Please open a separate bug.
> 
> I just submitted Bug 415591.
> 
> I should have this clearer on "this feature" to mean ignore-space.
> 
> Will ignore whitespace differences in compare viewer be supported soon?

Compare viewer accept either :
1. The old file contents + unified diff
2. Two file contents

If you use status page or commit page, that is the case 1. In this case the diff comes from server so I think as long as git diff support this param it shouldn't be hard.

If you compare any two files(by selecting two and use "compare with each other" action), that is case 2 where jsdiff.diffLines is used.

diffWords is used inside a pair of diff block.

I do not think we will support very soon but I still plan it for 4.0M2. I will think about it together with the ignore case feature after my vacation.
Comment 12 libing wang CLA 2013-08-22 10:20:49 EDT
One design/implementation suggestion:
Toggling "ignore white space" purely on client side.

On the UI side we will add an additional check box in the compare viewer.
By default it is not checked. When user checks it out, the compare viewer will filer out the white space on each diff block and re-render the diff annotations in the editor.
By doing this, we don't have to ask the server again in the git status page. Also we do not have to rely on jsdiff for white space filtering.
Comment 13 Adrian Aichner CLA 2013-08-22 10:26:47 EDT
(In reply to comment #12)
> One design/implementation suggestion:
> Toggling "ignore white space" purely on client side.
> 
> On the UI side we will add an additional check box in the compare viewer.
> By default it is not checked. When user checks it out, the compare viewer
> will filer out the white space on each diff block and re-render the diff
> annotations in the editor.
> By doing this, we don't have to ask the server again in the git status page.
> Also we do not have to rely on jsdiff for white space filtering.

Sounds good.

However, could this please be done in a way that one does not have to toggle the option on each individual file of a multiple file comparison.
Comment 14 libing wang CLA 2013-08-22 10:51:05 EDT
(In reply to comment #13)
> (In reply to comment #12)
> > One design/implementation suggestion:
> > Toggling "ignore white space" purely on client side.
> > 
> > On the UI side we will add an additional check box in the compare viewer.
> > By default it is not checked. When user checks it out, the compare viewer
> > will filer out the white space on each diff block and re-render the diff
> > annotations in the editor.
> > By doing this, we don't have to ask the server again in the git status page.
> > Also we do not have to rely on jsdiff for white space filtering.
> 
> Sounds good.
> 
> However, could this please be done in a way that one does not have to toggle
> the option on each individual file of a multiple file comparison.

We can still have the check box in each compare view, but we can add a user preference to control it globally. That says if you want the check box to be defaulted to true then check it in the user preference.
Comment 15 Adrian Aichner CLA 2013-08-22 11:05:37 EDT
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > One design/implementation suggestion:
> > > Toggling "ignore white space" purely on client side.
> > > 
> > > On the UI side we will add an additional check box in the compare viewer.
> > > By default it is not checked. When user checks it out, the compare viewer
> > > will filer out the white space on each diff block and re-render the diff
> > > annotations in the editor.
> > > By doing this, we don't have to ask the server again in the git status page.
> > > Also we do not have to rely on jsdiff for white space filtering.
> > 
> > Sounds good.
> > 
> > However, could this please be done in a way that one does not have to toggle
> > the option on each individual file of a multiple file comparison.
> 
> We can still have the check box in each compare view, but we can add a user
> preference to control it globally. That says if you want the check box to be
> defaulted to true then check it in the user preference.

I think this would be inconvenient.

You would have to navigate to your global settings just to make one multi-file compare operation ignore whitespace.

Then you would have to do it again to turn it off.

What I am asking for is to provide a top-level checkbox to switch all comparisons AND individual checkboxes for each file.

This would be similar to the [-] [+] (collapse/expand) buttons that are available, e.g. in git status, for all unstaged files as a whole, as well as individually for each file.

This lets the user choose in the fraction of a second with no need to visit any settings pages.
Comment 16 libing wang CLA 2014-01-13 10:02:12 EST
Bug 415591 will use the similar logic. I am setting this bug as a blocking one for that.
Comment 17 libing wang CLA 2014-02-25 11:52:32 EST
Priority in 6.0M1.
Comment 18 libing wang CLA 2014-09-08 18:05:44 EDT
Now I got time in 7.0 to implement this on client side.
Pushed the support in the compare viewer.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=c7d2627d107aee11a6105be5d64587de03472702


TODO:
1. Add more js unit tests.
2. Surface the checkbox in the comapre widget.
3. Add user pref to support a "global" flag like ignoreWhiteSpace.
4. Add more configurable details on how white space is ignored, like http://git-scm.com/docs/git-diff does.
Comment 19 libing wang CLA 2014-09-10 18:20:22 EDT
added ignore white space toggler in the compare widget.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=4d8205ad8aa36546bf61f7aef6f61a92f5738cfb
Comment 20 libing wang CLA 2015-01-29 10:05:59 EST
It is fixed now. In Orion if you toggle white space in one compare widget, the user preference is set. So next time, or if you open another compare widget, the white space setting is default to the one you set before.