Community
Participate
Working Groups
Created attachment 242323 [details] Concept of postfix templates This bug discusses the implementation/extension of the Eclipse template system to support postfix code completion. A summary of the concept of postfix code completion is described here: http://goo.gl/UdXIKF I have regularly used IntelliJ for the last two month to determine if postfix code completion enables major increase in productivity. The bottom line is that some of the postfix templates of IntelliJ really save a lot of time and key strokes. Especially the templates .var, .field, .for and some others lead to a very good developer experience. I'm aware of bugs like https://bugs.eclipse.org/bugs/show_bug.cgi?id=427201 and the discussed solutions using shortcuts for those purposes, but... 1.) Using shortcuts needs the developer almost every time to leave the home position (asdf jkl; respectively asdf jklö) of the keyboard which costs a lot of time in the long run and also disrupts the typing flow 2.) Remembering dozens of advanced shortcuts is hard 3.) Discovering advanced shortcuts is sometimes even harder Furthermore the existing template concept of Eclipse, to achieve auto code generation as described in the above-mentioned bug, is also inconvenient and not a time saving action (1. selecting the line, 2. invoking the content assist using Ctrl+Space, 3. selecting and applying the template using arrow keys) Summing up, I really would love to see postfix code completion in Eclipse. I am willing to implement this feature and therefore I have already digged into the sources of the template system of Eclipse and JDT. The attached image may be considered as a rough concept of postfix templates. I will try to implement a prototype plug-in which meets these requirements, hopefully without the need to touch existing code and I will come up with a prototype/proof of concept or at least new findings within the next week. Sidenote: Analyzing existing code completion techniques in general and Eclipse-specific techniques is the topic of my bachelor thesis, which is supervised by Marco Descher. A major part of the bachelor thesis also describes the implementation of a postfix code completion plug-in for Eclipse.
E.g. one of your examples has [person.getFriends()]. What if we have family.getWife().getFriends()? Would we always resolve the whole call chain for the variable or do you also intend to resolve to e.g. [family.getWife()] or [getWife().getFriends()].
(In reply to Dani Megert from comment #1) > E.g. one of your examples has [person.getFriends()]. What if we have > family.getWife().getFriends()? Would we always resolve the whole call chain > for the variable or do you also intend to resolve to e.g. [family.getWife()] > or [getWife().getFriends()]. As far as I understand the postfix completion evaluation is always done according to the object type return at the specific point of cursor. So in your case family.getWife().getFriends()_ where _ is the cursor position _ would resolve in a Collection type and "family.getWife().getFriends()" be the inner expression. I don't see the target here, that the inner expression itself is subdivided in order to get "alternative paths" like family.getWife(). It could be interesting, to support such scenarios, they might get pretty complicated however. Lets fix your example where it would be required to have a way to address the single sub-expressions in the statement like IF family.getWife().getFriends() IS ${inner_expression} we would need kind of a ${inner_expression_term}[0..n] with '.' being the splitters. Then one could iterate over these alternative paths. Is this what you meant?
(In reply to Dani Megert from comment #1) > E.g. one of your examples has [person.getFriends()]. What if we have > family.getWife().getFriends()? Would we always resolve the whole call chain > for the variable or do you also intend to resolve to e.g. [family.getWife()] > or [getWife().getFriends()]. Yes, the current concept would resolve the the whole call chain to the variable ${inner_expression}. As Marco already mentioned it would be nice to introduce some sort of index to use specific parts of the ${inner_expression} in the template, although I cannot think of a very common scenario which can't be solved using a combination of other postfix templates. Nevertheless I will keep your suggestion in mind.
(In reply to Nicolaj Hoess from comment #3) > (In reply to Dani Megert from comment #1) > > E.g. one of your examples has [person.getFriends()]. What if we have > > family.getWife().getFriends()? Would we always resolve the whole call chain > > for the variable or do you also intend to resolve to e.g. [family.getWife()] > > or [getWife().getFriends()]. > > Yes, the current concept would resolve the the whole call chain to the > variable ${inner_expression}. As Marco already mentioned it would be nice to > introduce some sort of index to use specific parts of the > ${inner_expression} in the template, although I cannot think of a very > common scenario which can't be solved using a combination of other postfix > templates. Nevertheless I will keep your suggestion in mind. I'm fine to start with the simple/suggested approach.
Finally I got a first prototype of a postfix code completion plug-in running in Eclipse. See the following video for a short demonstration and proof of concept: http://youtu.be/LfHJyW3AL08 The implementation, respectively the result, differs in two ways from the initial proposed concept: 1. The outer_expression variable is not used/resolved. After testing and defining some templates it turned out to be a pretty useless variable in practice. I was able to define the initially proposed template for creating a short if-statement also without this variable. In fact no template really needs the outer_expression variable. 2. The inner_expression resolves always to the most right completion node. Example (.| represents the position of invoking the CA): sysout(a + b.| => inner_expression is resolved to b In my opinion choosing the most right completion node is correct almost every time. I can only think of one scenario (parenthesizing an expression) in which I would like to choose the parent node. Are there other opinions on that? Overall the plug-in behaves pretty much the same as the IntelliJ postfix code completion and also supports all its features, except the .field template. However the biggest advantage in my opinion is, that the Eclipse implementation allows to define own postfix templates (i.e. like .nameconcat in the video), IntelliJ’s templates are hard coded. The implementation depends on some changes which were applied to the existing Template implementation of JDT UI. Those changes primarily affect access restrictions which would have led to some code duplication. One change also affects the CompletionEngine class in JDT Core which I had to make in order to allow completion on nodes which resolve to a base types, i.e. false.| or friends.size().| etc. In conjunction with Marco I will work the next days on setting up a test environment to test a possible productivity increase by using postfix code completion and will also do some testing regarding the performance of the invocation of the CA if the plug-in is active. I hopefully can also do some code review with Marco and then implement a new Resolver for a variable which resolves to a new field in order to support a .field template.
I like it very much! Could you point to your code here? Would like to have a look at it! Thanks for the great video!
Created attachment 242884 [details] Patch for JDT UI The patch includes all changes which were applied to JDT UI.
We will take a look at this during 4.5. Currently we are busy with the endgame for Luna.
Created attachment 242885 [details] Patch for JDT Core The patch includes all changes which were applied to JDT Core. The initial implementation prevented access to the ASTNode of the current completion if the node resolved to a base type. Without this change it is not possible to provide postfix completion assist in scenarios like: myList.size().| or false.|
Link to the source code of the plugin (it depends both patches attached above): https://github.com/trylimits/Eclipse-Postfix-Code-Completion
I have implemented a new variable "newField" which creates a new field/constant in the enclosing class, to support templates like: 1.) .field Template: ${field:newField(i)} = ${i:inner_expression};${cursor} Demonstration: http://abload.de/img/fieldstrp9.gif 2.) .constpriv and .constpub (very helpful imho, but not supported by any other IDE) Template .constpriv: ${n:newField(i, false, true, true, true)}${i:inner_expression(novalue)} Template .constpub: ${n:newField(i, true, true, true, true)}${i:inner_expression(novalue)} Demonstration: http://abload.de/img/debugconstvhu8u.gif http://abload.de/img/logconsttpu8m.gif (very common scenario in Android dev) The above "novalue" parameter is an extension to the inner_expression variable which simply prevents the output of the variable. This parameter is a workaround to nested variable declarations like "${foo(${bar})}" which are not supported by the template system. The parameters of the newField variable are defined as follows: newField(type, isPublic, forceStatic, isFinal, initialize) I'm not 100% happy with the fact that after a template is applied, the "override mode" is lost (see gif demos 2 and 3) which forces the developer to leave the keyboard's home position. Unfortunately I don't know if there is a way to reactivate the "override mode", but I will try to find a way.
(In reply to Nicolaj Hoess from comment #9) > Created attachment 242885 [details] > Patch for JDT Core Could you create a Gerrit review for this patch? See http://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html#eclipsegerritcontribution for the setup Reviewing and updating Gerrit review is typically much easier for both sides.
(In reply to Lars Vogel from comment #12) > (In reply to Nicolaj Hoess from comment #9) > > Created attachment 242885 [details] > > Patch for JDT Core > > Could you create a Gerrit review for this patch? See > http://www.vogella.com/tutorials/EclipsePlatformDevelopment/article. > html#eclipsegerritcontribution for the setup > > Reviewing and updating Gerrit review is typically much easier for both sides. Pushed to gerrit: JDT UI: https://git.eclipse.org/r/#/c/29586/ JDT Core: https://git.eclipse.org/r/#/c/29587/
Hy Dani, it would be very nice of you, if we could have a relatively quick review resp. merge of the proposed patches. Nicolaj will be available for a limited time-frame only and is preparing for the bachelor exam. After the patch is merged we will provide an update site in order to further work on the feature! Thank you very much
(In reply to Marco Descher from comment #14) > Hy Dani, > > it would be very nice of you, if we could have a relatively quick review > resp. merge of the proposed patches. Nicolaj will be available for a limited > time-frame only and is preparing for the bachelor exam. After the patch is > merged we will provide an update site in order to further work on the > feature! > > Thank you very much Sorry for the delay. Unfortunately still very busy. I've added my comments to the JDT UI change. Regarding the Core change I'm a bit hesitant with regards to side-effects for content assist. Moving to JDT Core to see whether all tests still pass and review the JDT Core change.
Interesting proposal. I have a comment/question on the JDT Core patch: Now with a proper selection found triggered, I see an additional call to findFieldsAndMethods (CompletionEnginer:2618). I am not an expert in this area, but are we expecting something from this call? If not, we can avoid this along with couple of ObjectVector instantiations. Am I missing something? BTW, all our tests passed - https://hudson.eclipse.org/platform/job/eclipse.jdt.core-Gerrit/148/
(In reply to Jayaprakash Arthanareeswaran from comment #16) > Interesting proposal. I have a comment/question on the JDT Core patch: > > Now with a proper selection found triggered, I see an additional call to > findFieldsAndMethods (CompletionEnginer:2618). I am not an expert in this > area, but are we expecting something from this call? If not, we can avoid > this along with couple of ObjectVector instantiations. Am I missing > something? I this related to this patch? I don't think the patch introduced this method call. > BTW, all our tests passed - > https://hudson.eclipse.org/platform/job/eclipse.jdt.core-Gerrit/148/
(In reply to Lars Vogel from comment #17) > I this related to this patch? I don't think the patch introduced this method > call. IIRC, with a proper completion node being sent, this was resulting in the code taking a different path in CompletionEngine#completionOnMemberAccess() and hence an additional call findFieldsAndMethods().
(In reply to Jayaprakash Arthanareeswaran from comment #18) > (In reply to Lars Vogel from comment #17) > > I this related to this patch? I don't think the patch introduced this method > > call. > > IIRC, with a proper completion node being sent, this was resulting in the > code taking a different path in CompletionEngine#completionOnMemberAccess() > and hence an additional call findFieldsAndMethods(). The method CompletionEngine#findFieldsAndMethods(..) already handles this case correctly and immediately returns if the passed TypeBinding is a base type, so in my opinion this method call should not be bypassed, should it?
(In reply to Nicolaj Hoess from comment #19) > The method CompletionEngine#findFieldsAndMethods(..) already handles this > case correctly and immediately returns if the passed TypeBinding is a base > type, so in my opinion this method call should not be bypassed, should it? Sorry about the delay. I went back to the problem and traced back my thought process for making that comment (comment #16). With the change in CompletionOnMemberAccess, the conditions for completion has been relaxed and as a result the code at CompletionEngine will be entered in one more scenario - i.e. when the type in question is also a base type. The findKeywords() call is guarded by the new check, but the code below that is not. If we are not sure the call to findFieldsAndMethods() serves any purpose in this particular scenario, we should avoid it, so things will be the way they were before.
(In reply to Jayaprakash Arthanareeswaran from comment #20) > (In reply to Nicolaj Hoess from comment #19) > > The method CompletionEngine#findFieldsAndMethods(..) already handles this > > case correctly and immediately returns if the passed TypeBinding is a base > > type, so in my opinion this method call should not be bypassed, should it? > > Sorry about the delay. I went back to the problem and traced back my thought > process for making that comment (comment #16). > > With the change in CompletionOnMemberAccess, the conditions for completion > has been relaxed and as a result the code at CompletionEngine will be > entered in one more scenario - i.e. when the type in question is also a base > type. The findKeywords() call is guarded by the new check, but the code > below that is not. > > If we are not sure the call to findFieldsAndMethods() serves any purpose in > this particular scenario, we should avoid it, so things will be the way they > were before. Thanks for the clarification. I have uploaded Patch Set 4 at https://git.eclipse.org/r/#/c/29587/ . The new implementation ensures that the extended context is built in CompletionEngine#complete(...) in the corresponding scenario and avoids additional method calls. I appreciate further feedback :)
I am reasonably convinced that the last patch uploaded doesn't affect the current code assist behavior. We can release whenever we have the 'go' for the UI patch.
(In reply to Jayaprakash Arthanareeswaran from comment #22) > I am reasonably convinced that the last patch uploaded doesn't affect the > current code assist behavior. > > We can release whenever we have the 'go' for the UI patch. AFAICS the UI patch has been merged by Dani. See https://git.eclipse.org/r/#/c/29586/ and https://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=2a4886c69e3ac30b76571020a4c1e87694106c8b
Released the jdt core part in master: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=75afdf2996a1c8ae6b0b5be7d9f9b9919d6b66a0
Created attachment 249280 [details] comment about the plugin
Verified for Eclipse Mars 4.5 M4 using build I20141208-0800 [Refer attachment 249280 [details] for a note about the plugin]
