| Summary: | Option to ignore whitespace differences in compare viewer | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Mark Macdonald <mamacdon> |
| Component: | Client | Assignee: | libing wang <libingw> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | adrian.aichner, eclipse.felipe, evan_hughes, libingw, Silenio_Quarti, simon_kaegi, tomasz.zarna |
| Version: | 0.3 | ||
| Target Milestone: | 7.0 | ||
| Hardware: | PC | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 415591 | ||
|
Description
Mark Macdonald
+1 Please consider this feature for the next release 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. (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. (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? For the Jazz case we'd rather rerun the diff on the server side. I really miss this feature. 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? I should have mentioned that diffWords should give as the feature asked for in this feature request. (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. (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? (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. 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. (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. (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. (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. Bug 415591 will use the similar logic. I am setting this bug as a blocking one for that. Priority in 6.0M1. 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. added ignore white space toggler in the compare widget. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=4d8205ad8aa36546bf61f7aef6f61a92f5738cfb 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. |