Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add method override autocompletion for Groovy Plugin #239

Closed
wants to merge 2 commits into from

Conversation

jmartinesp
Copy link
Contributor

This was created as a separate plugin in https://plugins.jetbrains.com/plugin/7571,
but it fits better inside the official Groovy plugin.

This was created as a separate plugin in https://plugins.jetbrains.com/plugin/7571,
but it fits better inside the official Groovy plugin.
@klikh
Copy link
Member

klikh commented Jan 30, 2015

Have you submitted a contributor license agreement? If not, please download the PDF from http://www.jetbrains.org/display/IJOS/Contributor+Agreement and then print it out, sign (mention you github account name there), scan and email to contribute@jetbrains.com

@jmartinesp
Copy link
Contributor Author

I got accepted as a contributor this morning. Thank you.

@klikh
Copy link
Member

klikh commented Feb 2, 2015

Agreement received. Thanks.

@maxmedvedev
Copy link
Contributor

Hi Jorge!
This is really cool. I'll accept the pull request after some improvements.

protected void addCompletions(@NotNull CompletionParameters parameters, ProcessingContext context, @NotNull CompletionResultSet result) {
PsiElement position = parameters.getPosition();

if(psiElement()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to check psiElement().inside(PsiClass.class) since we search for containing class and check if it is not null on the next line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done because otherwise you could get override or implement suggestions while inside a method definition, which I don't think really make sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

psiElement.inside(PsiClass.class) is fully equivalent to PsiTreeUtil.getParentOfType(position, PsiClass.class) != null. Right now completion inside method body suggests overriding variants.

The correct place to put the pattern is register method where currently you provide PlatformPatterns.psiElement(PsiElement.class). See a describing comment there

@maxmedvedev
Copy link
Contributor

Don't worry about tests. Not a big deal actually. I'll write one or two examples to guarantee the feature won't be broken in future.

@maxmedvedev
Copy link
Contributor

Sorry for the delay. We will accept your commit in a few day. Thanks!

@jmartinesp
Copy link
Contributor Author

That's really nice to know! I have a question, though: if I wanted this to be included on the next version of Android Studio should I add another PR to the 135 branch? Thank you 👍

@maxmedvedev
Copy link
Contributor

If you want we can try to cherry-pick this to 135. But are you sure Android Studio uses the 135 branch?

@jmartinesp
Copy link
Contributor Author

It is, as far as I know. The about me dialog shows 135 as build number and this answer in SO seems to support that.

@jmartinesp
Copy link
Contributor Author

Hi, is there any way I can help to make this PR merge? It's been almost a month since the last update.

@dovchinnikov
Copy link
Member

Merged manually with updates.
@Arasthel actually it was like year ago, sorry for no notification.

SergeyZh pushed a commit that referenced this pull request Oct 9, 2020
fixed #754 and #239

GitOrigin-RevId: e470f5d0cf6840502eced6c236101391e32ad54c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants