Community
Participate
Working Groups
I've added several new useful (at least for me) quick assist's in class QuickAssistProcessor (from 3.0 release). Here is list of them: 1. Split condition. For example you have following code: if (condition_1 && condition_2) { do_something2(); } Now you decide that you need also execute do_something3() when condition_1 && condition_3. I.e. you want following code: if (condition_1) { if (condition_2) do_something2(); if (condition_3) do_something3(); } Currently you will need to copy condition_2 by hands. I don't know how all other, but I am very lazy. With new quick assist you can position caret on condition (for example on &&) and transform initial code in: if (condition_1) { if (condition_2) do_something2(); } 2. Join 'if' statements. This is reverse operation to "split condition". You can point on inner or outer "if" statement and ask for joining. Depending on context, quick assist will suggest to join outer "if" to inner or inner to outer, or both, if possible. Join can be used only if there are no "else" parts in "if" statements. 3. Add paranoidal parenthesis for conditions. I don't like to look on conditions and think "And what is order of executing these conditions?", so usually add parenthesis around each expression like: "collection.size() == 1" or (object == null), etc. However sometimes I have to change someone else code and would like to change source to make it more readable for me. So, I've added quick assist for adding parenthesis in such cases. For example this: list.size() == 3 && list instanceof List && c will be converted to: (list.size() == 3) && (list instanceof List) && c Just select block of code and invoke quick asisst, it will change all conditions in selected code. 4. Remove extra parenthesis. One more reverse operation. If you don't like extra parenthesis in your code, you can remove all of them and keep them only where required. For example this: (list.size() == 2) && (a || b) will be converted to: list.size() == 2 && (a || b) As you can see, it keeps parenthesis where needed. It uses precedence table for operators, so knows, where to keep parenthesis. 5. Inverse condition. Select condition and inverse it, for example when you see that there are too many "not" operators or want to exchange "then" and "else" branches in "if" statement (BTW, may be such quick assist also required?...). For example, this: a || !b && !c will be converted to: !a && (b || c) BTW, I often use what I call "fail fast" checking. I.e. when you do chain of checks, each next can be executed only if previous were successful, instead of writing deep chain of "if" statements: prepare_1; if (condition_1) { prepare_2; if (condition_2) { prepare_3; if (condition_3) { ... } } } I prefer to have prepare_1; if (!condition_1) return false; // or may be 'continue' prepare_2; if (!condition_2) return false; prepare_3; if (!condition_3) return false; ... So, condition inversing could be useful to implement assists for changing from one style of chain checking to another... I would like to have these quick assists in Eclipse, so would like to contribute them to Eclipse.
Created attachment 13703 [details] Patch for QuickAssistProcessor
This Patch seems to implement the request of Bug 6605.
Not exactly. This patch has only quick assist for reversing condition inside of 'if' statement, not exchanges 'then' an 'else'. But I agree, that it is easy to implement such asisst using existting now functionality. I will try to implement case when both 'then' and 'else' exists tonight. But do you have idea, what to do when only 'then' exist? Move this 'then' to 'else' and create empty 'then' statement? Another possibility here is my "fail fast" style, i.e. if current 'if' statement is last statement in 'for' or 'while' we can use 'continue' as 'then' statement for this reversed 'if'. And resever is also possible, when we reverse 'if' with 'continue' as 'then', we can move rest of cycly in 'then' block of inversed 'if'. More ideas?
Currently no more ideas from my side - it's already more than I would have asked for. ;) Of course once I actually use that feature in daily work, new ideas might emerge...
I still hope that these quick asissts can be included in Eclipse, but want to "break in" them as separate plugin. It was published (with source) here: http://eclipsecolorer.sourceforge.net/advanqas/index.html
Another idea: quick assists to toggle between if (someCondition) { bar(); return; } foo1(); foo2(); and if (someCondition) { bar(); } else { foo1(); foo2(); }
What is after foo2() in original method? Nothing, end of method? I.e. if there is if (condition) { bar(); return; } and this 'if' statement is directly in method body, place rest of method body in 'else' statement? AFAIK there is optional warning in Eclipse Java compiler for 'unnecessary else statement'...
Yes, exactly - put the rest of the method in the else block *and remove the return statement from the if block*. In that case, you don't get a warning for an unnecessary else.
I was on vacation for 6 weeks, so excuse the silence. Of course I'm interested in integrating these quick assist if you donate them!
Do you want to update the patch or is it still up to date? I would put them all in a separate processor. I'll also need to add test cases, you don't happen to have that as well?
Created attachment 14350 [details] Advanced quick assists version 1.3 This patch containt archive for separate plugin 'AdvanQas' that I've used to add users ability to use my quick assists and find bigs in it. It has already separate processor, so I hope it will be more easy to integrate into Eclipse.
Ok, now 'patch' is up to date. Yes, you are right, I don't have yet test cases, but I am ready to implement them. I am sure you have test cases for current set of quick assists, can you point me on them so, I will able to use them as example?
released AdvancedQuickAssistProcessor in package org.eclipse.jdt.internal.ui.text.correction. > 20040902 Did a quick rewiew, looks very good, nice work! Thanks a lot for your help! - Added copyright statement, applied minor changes (getCoveringNode(context) -> context.getCoveringNode()) - Removed the add then/else proposal and incorperated it in the existing addBlock quick assist (QuickAssistProcessor). - I disabled the 'getVariableDebugOutputProposals'; they are very intrusive, say, they show up at too many places. We should discuss what we do here. For tests have a look in plugin org.eclipse.jdt.ui.tests, 'AssistQuickFixTest'. I mark this bug report as fixed, but please feel free to use it for discussions and suggestions.
I created AdvancedQuickAssistTest.java with some first tests.
Wow, cool. I not expected that you will add my code so fast. :-) Thank you for sample for new test cases, I will try to add tomorrow test cases for several quick assists. BTW, as I can see testSplitIfCondition1() and testSplitIfCondition2() are almost same, I think you added them only to show me, how to implement test cases, right? And I see one possible way for simplification. As I can see, you check result for all possible quick assist in selected position. IMHO this is not very useful. We need check only one quick assist, but we don't know its position in list. So, why not ask each proposal for result and check that one of them is expected result. Do you see problem with such method of testing?
Yes, testSplitIfCondition2 was intended to be more different. I git distracted while writing and forgot later. The simplification you suggest is ok for me. In fact it makes a lot of sense for quick assist where several assists interfere wich each other. I added a method 'assertExpectedExistInProposals' to the quick fix base class and changed 'testInverseIfCondition1' to use it. The orginal kind of testing has the advantage that you can also keep an eye on how the several quick assist interact and how many you get for a certain situation. I'm trying to prevent that the list of assists and quick fixes gets too big. That was BTW the reason why I commented 'getVariableDebugOutputProposals' out. Talking about 'that debug output assist. Do you strongly want it? What we could do is to force the user to do a full selection of the variable (or expression) so it shows up less, but of course that makes it harder to detect.
Thank you for 'assertExpectedExistInProposals' :-) About debug output - this was originally request from one of users of AdvanQas, but I use it myself. In reallity this quick assist already can display selected expression, so this is not much implementation problem. But this will make it not so easy to use. We could also add option in preferences, in which cases to show this quick assist, for example following options: when cursor on left side of assignment, when cursor on method argument, when expression selected, when cursor is on any expression. Well in any case, I not insist on this quick assist, I always can use it in separate plugin. BTW, I am trying to start JUnit plug-in tests, but always have error: "Error: " java.lang.IllegalArgumentException: No ClassLoader found for testplugin: org.eclipse.jdt.ui.tests Do you know how to fix this? I use "JUnit Plug-In Test" layoun group and already provided someone "org.eclipse.jdt.ui.tests" launch configuration. Eclipse Build id: 200409011200. May be PDE JUnit is broken in this version?...
200409011200 works for good me. I don't use this prepared launch configuration. Just select the class with the tests and run as 'JUnit plugin-Test'. To run all Quick Fix tests run on 'QuickFixTests'. To run all jdt.ui tests run on 'AutomatedTest'. It's also possible to run just a single tests, select the test method for that. Maybe mail me directly if this wasn't the problem.
Another idea for a quick assist: switch between if-else and the ternary operator. That is: if (b) return a; else return b; <=> return b ? a : b; and if (b) v=a; else v=b; <=> v = b ? a : b;
Even more ideas can be found in Bug 23261, Bug 38018 and Bug 68743. Bug 50255 and Bug 68742, on the other hand, are already implemented by this patch, so it seems to me.
Ah, sorry, in comment #19, the b in the condition is not the same as the b assigned/returned, of course :rolleyes:
*** Bug 50255 has been marked as a duplicate of this bug. ***
*** Bug 68742 has been marked as a duplicate of this bug. ***
Thanks for the research, Ilja!
You're welcome. Actually I just stumbled across those while trying to find this bug report again... ;)
I like these ideas. Here I will summarize them: 1. if return else return <=> ternaty. Here additional idea is that we could do same thing when assignment to same variable presents instead of return. 2. if else if ... <=> switch 3. 'push negation in' and 'pull negation out'. Here question is how to specify, where we want to do such trasformation. For example - should user select only one expression and do changes only in this expression, or may be select block of code and move all negations in all boolean expressions as up as we can?... Hm... Most probably first. Any ideas? 4. inverse local boolean variable with value assigned only in one point, so right side of assignment should be inversed and each use of variable should be done with negation (or remove negation if it is already used). If nobody else wants to implement these quick assists, I could do this first in AdvanQas (and remove old quick asists that already in Eclipse :-)) to test them. Or, may be, add them directly in Eclipse code? I just think that Eclipse has so much of users and I don't want to break Eclipse because my plugin does something wrong... Well, in any case I will send patch to Martin. :-) Martin, how do you think?
Created attachment 14607 [details] New quick asists 1. getInverseConditionalExpressionProposals: bool ? a : b <=> !bool ? b : a 2. getExchangeIfStatementConditionsProposals: Exchanges inner and outer 'if' statements conditions: if (a) { if (b) { do_something(); } } into: if (b) { if (a) { do_something(); } } 3. getExchangeInfixOperatorOperandsProposals: Exchange left and right parts of infix expression in case of commutative operator ( +, *, &, |, && ||, ^ ). 4. getCastAndAssignIfStatementProposals: In many cases directly after checking type we need to use some expression as just checked type, so we need to declare variable of this type. This quick assists simplifies process. If there is code: if (exp instanceof Type) { } and press Ctrl+1 of 'if', quick asisst suggests to create following code: if (exp instanceof Type) { Type type = (Type) exp; }
fyi: Martin is away again. He will be back Oct 4.
Ok, so I should be patient. :-)
Verified 1. - 5. in I200409212000. Good work! My favourites are "Remove extra parenthesis" and "Inverse condition".
I suggest to finish discussion in this bug report and open new one(s) for new quick assist(s). Otherwise it gets hard to track target milestones i.e. since when is which quick assist available and has it been verified.
Ok, I open new bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=74746 with new quick assists that were described in comment 27.
