IllegalArgumentException: Group not found: diffLabel #1295
Comments
|
This seems to be a regression of #731 |
|
Simpler way to reproduce: Team > Apply Patch using a patch with many conflicting changes.
This situation is more severe, because the compare editor is broken henceforth, and selecting other changes does not bring it back! As I was about to lose an hour's work resolving many conflicts in a monster patch, I had to be inventive, and luckily switching from "Java Source Compare" to "Text Compare" gave me a working compare editor, from where I could also go back to "Java Source Compare". |
I am debugging this case. Currently I can understand that all contributions are empty for toolbar manager during this call ! |
|
The issue seems to be in setInput method of CompareViewerSwitchingPane.java where during this call a proper viewer for the given input is not found and a NullViewer is assigned . Toolbar is cleared out in the NullViewer constructor. I still need to check on fixing part of this issue ! Any suggestions are welcome |
|
The file that needs fix is TextMergeViewer.java which is part of eclipse.platform |
What about this: Unfortunately steps to reproduce seem to be non trivial, if that would be easier way I could test if that patch works or not. |
|
Arrgh, this commit 5c071d3 added lot of save actions that make it impossible to produce a clean patch. @HeikoKlare : I just wanted to change few lines but extra actions make it impossible to keep only related changes. Can we please undo this and do not perform so much changes automatically? @lathapatil : here is what I really wanted to propose: |
The unintended changes only seem to be related to combining if clauses (just like in #1282) and replacing anymous classes with lambdas. So if this is a recurring problem (which it seems to be), we should probably turn off specific autosave action(s). I would not be in favor of completely reverting the commit completely, as it streamlined all the different, inconsistent autosave configurations across the platform projects (and basically only applied the JDT default settings). This streamlining would completely be lost when reverting the commit, while a follow-up fix would be relatively easy.
Of course, the latter may produce changes in existing code, but they should be far more local than the if-clause changes (and they are reasonable in almost every case). Note that you can also easily revert the auto save actions by performing an undo and re-save after doing the save that applied the auto save actions. What do you think, @iloveeclipse? I will at least provide a PR disabling the if-clause-related save action soon. |
Yes, please. If anyone wants to have a cleanup change, one can run whatever cleanups wanted. However it is impossible to create a single line change without searching & reverting related settings on the project. |
|
Another possibility would be to run all the wanted cleanups before enabling actions by default, so if something is changed, it is only in the new code. |
Yes, that has also been discussed in #1001 and #1282. I just did not put any urgency on it as I have not noticed many PRs suffering from save actions applying unexpected changes.
Note that is is not about overall cleanups but about save actions. So it is basically about rules to follow when contributing new code. An essential problem is that save actions cannot apply to changed code but always apply to a whole file. I will now go through the rules and either apply an according cleanup making all code conform to the save action rule or remove the save action rule again, so that save actions on existing code will always be a no-op. |
Below empty and null checks fix the current issue . If the toolbar is empty there is no need to show the differences as well.
|
Not sure whom you ask? If your patch fixes the problem (it seem to), please set a PR. Please note, one should avoid the extra async call below (that updates toolbar) if no changes were made (like in my patch proposal) |
@iloveeclipse Soon I will set a PR for the fix .
Apologies, I didn't understand the code before because I read it as a whole instead of viewing it as a diff. I'll adjust my approach to prevent making async calls if no changes are made to the toolbar. |
I've been getting this IllegalArgumentException for a little while now. Today I found a corresponding stacktrace in the log.
This time I was in the "Encapsulate field... " refactoring, had inspected the preview of a particular class and then pressed back. This scenario reliable reproduces the bug for me.
There have been various other situations for the same problem, too.
The text was updated successfully, but these errors were encountered: