Community
Participate
Working Groups
Bug 292831 introduced a label notifying a user that comparison results might be inaccurate due to algorithm optimization. User should be able to disable the capping on demand. The way to go is making the message a link or a button.
I just figured out all the discussion in Bug 292831 was about showing a message. I'm sorry that the message doesn't help in our case (bug 303533). Our customer didn't want to know why the comparison was not a good one. He wanted a right result to be displayed. Any target date for this bug?
Would the solution suggested by Pawel in comment 0 work for you and your customer? In case when a faster comparison algorithm has been used, you are informed about that with the message added in bug 292831 and then you will be able to run the comparison one more time, without any capping, by clicking on a link or a button next to the message. If this is not acceptable in your case, I will need to ask to read all of the comments from bug 292831 and look for a solution that suits you best. I'm sure guys have considered all possible approaches in their comments and we don't want to go through that again ;) Personally, I like the solution from comment 0.
Note that the uncapped algorithm can take minutes and block the UI without any way to stop it. I doubt that anyone would like to see this happen. People would probably think that the workbench died, especially since no progress is reported. So, before simply starting the uncapped algorithm, we would have to make sure the algorithm can be canceled. Even better would be to report progress in such cases.
Either a link or button should work fine. Wonder whether I can ask for a preference setting for this feature. The comparison can be a optimized one by default. The user can change it to the full text comparison to avoid clicking the link/button again and again. For the UI lock part, I can not agree more that a way to cancel the comparison should be provided. And I hope the locked part is only the compare editor itself. The rest of the UI should be still available to the user during the comparing.
(In reply to comment #3) > So, before simply starting the uncapped algorithm, we would have to > make sure the algorithm can be canceled. Even better would be to report progress > in such cases. I totally agree. I thought we already have a bug for that, but couldn't find it, so I filed a new one -- bug 322329. Once the blocker is fixed, we can proceed with adding a link or button (this bug). Regarding the preference, Pawel already suggested that in bug 292831, comment 11, so please see Dani's answer why this is not a perfect solution.
It seems that comment 35 made by Dani is the final proposal. And He suggested preferences page as the item (e). Was I reading correctly?
Well, yeah. He did mention the preference, but it was considered as a last resort. Just after saying that, Dani suggested a solution which we had agreed upon. In the proposition he said we should "enrich the message with either a link or button that allows to run the slower algorithm" (comment 0) as soon as we start to support cancel in the diff algorithm (bug 322329). I'm citing Dani and comments from bug 292831 here to emphasize that we've already came up with a solution which in our opinion is the best. The other thing is that we also agreed that "As a final solution later on it needs to be possible to compare such large files with accurate results" (bug 292831, comment 47), but this is something we most likely we'd *not* want to backport. I guess we will have to revisit that conclusion now.
>I guess we will have to revisit that conclusion now. Right. The code to cancel the comparison plus the new UI and workflow are probably too big and risky for a maintenance fix. In addition it would add new strings which would not be translated for non-English users.
Can you please advise what's the current design for this fix and what's the next plan? There are a lot of discussion in bug 292831 and it's not clear what's going to be done. By the way. from an end user's point of view, I'd rather to have a slow and accurate result. If I have a big file, I know exactly what is going to happen when I deal with it. And if I'm comparing two files, my goal is to find the difference, not to save the time. As long as a cancel button exists, I can always cancel the comparison if I can not bear it.
>By the way. from an end user's point of view, I'd rather to have a slow and >accurate result. Yep, agree, but so far without the cancel fix it meant that you might have to wait for minutes or hours without progress, the UI blocked and no way out except killing the workbench. I don't think that you'd want that. >Can you please advise what's the current design for this fix and what's the >next plan? For 3.7 we will allow to cancel and then offer *some* way to choose the slower algorithm.
Dani's story is scary. I wonder how big the file is. I'm frequent user of Beyond Compare and I never see a single file compare takes more than a few seconds. I have a file of 7000 lines and 250KB which Eclipse compare can't handle properly. It took Beyond Compare a split second to do the job. If eclipse's accurate algorithm needs hours to finish the job, we may want to open another bug to address the performance issue. When Dani talked about GUI is locked, I hope it's just the compare editor. There is no reason to lock the other part of the perspective. I can't image a fix without a cancel button. Can someone advise what's going to be fixed in 3.6.1?
It will not be the common case that it takes as long as I outlined and it's not just the size of the files but also what the changes are. >When Dani talked about GUI is locked, I hope it's just the compare editor. >There is no reason to lock the other part of the perspective. I *think* it depends how you open the compare editor, but Tomasz would know better. >I can't image a fix without a cancel button. Can someone advise what's going to >be fixed in 3.6.1? Currently the plan is to fix it in 3.7 as the changes will most likely be to big/risky for 3.6.1. If the 3.7 fix has been in use for a while one could discuss to backport it into 3.6.2.
Created attachment 176536 [details] Initialzing the editor and computing diffs (In reply to comment #12) > >When Dani talked about GUI is locked, I hope it's just the compare editor. > >There is no reason to lock the other part of the perspective. The job that initializes and opens the editor can be run in the background, it's non-blocking. The problem is with the diffing algorithm which runs in a *modal* dialog. To make matters worse, the dialog cannot be cancelled and this is what I'm trying to solve in bug 322329. If you have any questions about that, please comment of that bug. This bug is about providing a way to choose the slower (uncapped) algorithm.
Created attachment 176766 [details] dirty fix Here is a dirty fix which I decided to attach only to show how the solution from comment 0 could work. Do not commit!
Created attachment 176767 [details] mylyn/context/zip
Created attachment 178749 [details] Fix v01 Fix that adds new option to Compare preference page. The option is actually stored in org.eclipse.compare.code node, so it can be accessed by the code plug-in. However, since the pref page is tightly coupled with OverlayPrefernceStore the option is also stored in the UI plugin which is redundant.
Created attachment 178750 [details] mylyn/context/zip
I would suggest the following changes: 1) Mnemonic 'D' is already used, we could use 'C' instead. 2) Leave #cappingDisabled and its get/set methods in the core plug-in, but don't store its value in the pref store. The only place to store the capping preference value should be UI (as it is already done). 3) In DocumentMerger#doDiff call ComparePlugin#setCappingDisabled before diffs calculation is performed.
Thank you for fixing the problem. I wonder how I can pick up a driver to test it.
Created attachment 179249 [details] Fix v02 1) and 2) applied. As for 3) I decided to set the flag in ComparePlugin (core) on each preference change and on CompareUIPlugin (UI) startup. I did so because DocumentMerger#doDiff is not the only place where I should set the flag and by setting it as in the patch I'm sure it's properly init'ed in the core bundle.
Fix v02 applied to HEAD. Available in builds >=I20100921-0800. Samuel feel free to pick up tomorrow's integration build and verify both fixes. Please, keep in mind that we still haven't fixed bug 324993.
fix verified
Sam, have you had a chance to verify the fix as well?
The build I can find on the eclipse site are I20100922-0800 and I20100921-1024 for 3.7 stream. Both are marked with a red cross for windows 3.2. Which one should I try?
The red cross indicates that some tests have failed. The builds are still good from our perspective. Please, pick any of them.
I picked up eclipse-SDK-I20100922-0800-win32.zip and it works fine. And I have to say it's pretty fast as well. I really appreciate it.
There is also bug to provide documentation to this functionality. This is bug 326678
