Summary: | Change the default direction of the compare editor by default | ||
---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Marc-André Laperle <malaperle> |
Component: | Compare | Assignee: | Lars Vogel <Lars.Vogel> |
Status: | NEW --- | QA Contact: | |
Severity: | enhancement | ||
Priority: | P3 | CC: | akurtakov, alvaro.sanchez-leon, christian.dietrich.opensource, daniel_megert, dh_tue, eclipse.sprigogin, gautier.desaintmartinlacaze, Lars.Vogel, loskutov, marc.khouzam, markus.kell.r, patrick.pollo.guilbert, pyvesdev, sravankumarl, steffen, stephan.herrmann |
Version: | 4.7 | Keywords: | noteworthy |
Target Milestone: | --- | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://git.eclipse.org/r/148467 | ||
Whiteboard: | |||
Bug Depends on: | 213780 | ||
Bug Blocks: |
Description
Marc-André Laperle
2016-09-19 14:02:08 EDT
+1 for switching the default +1 In our company we deployed Eclipse with the default preference flipped and users seem happy with it. +1 +1 +1 for Switching defaults and keep the switch in the preferences (In reply to Alvaro Sanchez-Leon from comment #5) > +1 for Switching defaults and keep the switch in the preferences Yep, the preferences switch is a must. There will be always people used Eclipse for 10+ years and never used any tool where the direction is "wrong" (if coming from this point of view). Product owners will also need to make sure their products go fine with the change. Marc-Andre or Sergey: do you plan to provide a patch? Just in case we don't get a -1 from Dani :-) We should do this (if no one disagrees) as early as possible so that we can catch as much feedback as possible during 4.7 release cycle. (In reply to Andrey Loskutov from comment #6) > Marc-Andre or Sergey: do you plan to provide a patch? Just in case we don't > get a -1 from Dani :-) I intend to write a patch if nobody vetoes this change. +1 I'm still curious to know why eclipse shows the new version on the left. All other diff tools I know show it on the right. Anyone? (In reply to Marc Khouzam from comment #8) Let's focus on the bright future, not the ugly past :-). Do you have an idea, how to signal to users, that sides have swapped? I'd argue that *every* existing Eclipse user will be very surprised and make mistakes at first, UNLESS clearly informed about the change. (I had a number of such accidents 2 months back - not happy). +1 (In reply to Stephan Herrmann from comment #10) > Do you have an idea, how to signal to users, that sides have swapped? > > I'd argue that *every* existing Eclipse user will be very surprised and make > mistakes at first, UNLESS clearly informed about the change. Very good point. I'm not aware if there is any possibility to show a warning header in the diff editor (with a link to disable it), but this would be one possible solution. Another one (definitely less effort) would be to show a popup first time the compare editor is shown in the current installation, for 4.7 release only, with some explanation that diff order is different now. +1 -1 for simply changing the default, but +1 for a solution where the user either can choose upfront or has a very easy way to switch back to the long existing default. (In reply to Dani Megert from comment #14) > has a very easy way to switch back to the long existing default. That's already there with the toolbar button, but what's needed is to make the user aware, that the default has changed (see comment 10). (In reply to Dani Megert from comment #15) > (In reply to Dani Megert from comment #14) > > has a very easy way to switch back to the long existing default. > > That's already there with the toolbar button, but what's needed is to make > the user aware, that the default has changed (see comment 10). The best solution in my view would be an interstitial popup shown once in the Compare editor. Does anybody know of existing interstitial popups in Eclipse? If not, I'll file a bug for creating them. (In reply to Sergey Prigogin from comment #16) > (In reply to Dani Megert from comment #15) > > (In reply to Dani Megert from comment #14) > > > has a very easy way to switch back to the long existing default. > > > > That's already there with the toolbar button, but what's needed is to make > > the user aware, that the default has changed (see comment 10). > > The best solution in my view would be an interstitial popup shown once in > the Compare editor. Does anybody know of existing interstitial popups in > Eclipse? If not, I'll file a bug for creating them. The Compare editor has some code that allows to show a warning (e.g. to switch to another compare algorithm). Maybe that can be (re-) used. (In reply to Dani Megert from comment #17) > (In reply to Sergey Prigogin from comment #16) > > (In reply to Dani Megert from comment #15) > > > (In reply to Dani Megert from comment #14) > > > > has a very easy way to switch back to the long existing default. > > > > > > That's already there with the toolbar button, but what's needed is to make > > > the user aware, that the default has changed (see comment 10). > > > > The best solution in my view would be an interstitial popup shown once in > > the Compare editor. Does anybody know of existing interstitial popups in > > Eclipse? If not, I'll file a bug for creating them. > > The Compare editor has some code that allows to show a warning (e.g. to > switch to another compare algorithm). Maybe that can be (re-) used. Thanks for the hint, I've just stumbled over this again: this is unfortunately hardcoded via labelOptimized and recomputeLink in org.eclipse.compare.internal.CompareContentViewerSwitchingPane. If someone wants to add something like this extra buttons here, one can either try to generalize them (which would mean we will still have only one info and we will need a decision which from many infos we want show) or change the layout so that we can have multiple "info + action" lines in the header, or provide something like "Multiple warnings: please hover over to see all" label. *** Bug 549668 has been marked as a duplicate of this bug. *** If it's a temporary warning in place for 4.13 but removed in 4.14, why not, but it's not ideal: if users migrate from 4.12 (or older) to 4.14 (or newer) without going through 4.13, they wouldn't be covered. Making the warning permanent is probably even worse in my opinion: it's the default behaviour in pretty much any other tool, warning about something that's natural/expected is confusing. As I'm guessing any warning/popup would require more work (which explains why this has been stalled for several years now?), would the pragmatic option be to just go ahead and change the default? True, in the short-term some users may be surprised at first, but the benefits in the mid/longer term outweigh this in my opinion. Clearly communicating about it (N&N, Holder's new release videos, Tweeting, etc.) would go a long way about informing the users beforehand and minimising the impact. (In reply to Pierre-Yves B. from comment #20) +100 I hope, Dani is reading this. (In reply to Sergey Prigogin from comment #21) > (In reply to Pierre-Yves B. from comment #20) > > +100 > > I hope, Dani is reading this. Maybe the next step is to prepare a patch to make progress on this and keep the discussion going? (In reply to Pierre-Yves B. from comment #22) > Maybe the next step is to prepare a patch to make progress on this and keep > the discussion going? Yes, please provide Gerrit. Please let me know if you need help with Gerrit. Sergey, you wanted to provide a patch of I'm not mistaken? I won't have access to a proper dev environment in the next couple of weeks. The PMC agree that we try out the switch in 4.14 M1. See https://wiki.eclipse.org/Eclipse/PMC New Gerrit change created: https://git.eclipse.org/r/148467 (In reply to Eclipse Genie from comment #26) > New Gerrit change created: https://git.eclipse.org/r/148467 When I ran locally I got these failures. Also one of the test thrown a popup and it is waiting for user input. I think that is the reason for timeout testModifyRight(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.528 s testCopyEmptyLeftToRightAndModify(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.47 s <<< ERROR! java.lang.NullPointerException at org.eclipse.compare.tests.TextMergeViewerTest.testCopyEmptyLeftToRightAndModify(TextMergeViewerTest.java:352) testCopyRightToEmptyLeft(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.528 s testCopyRightToLeft(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.436 s <<< FAILURE! org.junit.ComparisonFailure: expected:<[hi there]> but was:<[some text]> at org.eclipse.compare.tests.TextMergeViewerTest.testCopyRightToLeft(TextMergeViewerTest.java:242) testCopyEmptyLeftToRight(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.407 s <<< FAILURE! java.lang.AssertionError: expected null, but was:<org.eclipse.compare.tests.TextMergeViewerTest$EditableTestElement@3bc20984> at org.eclipse.compare.tests.TextMergeViewerTest.testCopyEmptyLeftToRight(TextMergeViewerTest.java:287) testCompareFilter(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 20.335 s testDocumentAsTypedElement(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.329 s testModifyLeft(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.197 s testCopyLeftToEmptyRight(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.42 s testCopyEmptyRightToLeftAndModify(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.34 s <<< ERROR! java.lang.NullPointerException at org.eclipse.compare.tests.TextMergeViewerTest.testCopyEmptyRightToLeftAndModify(TextMergeViewerTest.java:338) testCopyEmptyRightToLeft(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.528 s <<< FAILURE! java.lang.AssertionError: expected null, but was:<org.eclipse.compare.tests.TextMergeViewerTest$EditableTestElement@6c96403e> at org.eclipse.compare.tests.TextMergeViewerTest.testCopyEmptyRightToLeft(TextMergeViewerTest.java:299) testCopyLeftToRight(org.eclipse.compare.tests.TextMergeViewerTest) Time elapsed: 0.451 s <<< FAILURE! org.junit.ComparisonFailure: expected:<[hi there]> but was:<[some text]> at org.eclipse.compare.tests.TextMergeViewerTest.testCopyLeftToRight(TextMergeViewerTest.java:254) Thanks Sravan, I will have a look (In reply to Lars Vogel from comment #28) > Thanks Sravan, I will have a look Lars, do you plan to work on that? (In reply to Andrey Loskutov from comment #29) > Lars, do you plan to work on that? I'm currently busy with customer work, so if the above means that you want to take over, that is fine. Otherwise, I will pick this up, once I have again time (> 3-4 weeks). Removed milestone as no one reacted to https://www.eclipse.org/lists/eclipse-dev/msg11217.html. |