jira.codehaus.org

  • Log In Access more options
    • Online Help
    • Keyboard Shortcuts
    • About JIRA
    • JIRA Credits
    • What?s New
  • Dashboards Access more options (Alt+d)
  • Projects Access more options (Alt+p)
  • Issues Access more options (Alt+i)
Signup
GRECLIPSE
  • GRECLIPSE
  • GRECLIPSE-909 Groovy Quickfixes in the editor
  • GRECLIPSE-936

Better ordering for quick fixes

  • Log In
  • Views
    • XML
    • Word
    • Printable

Details

  • Type: Sub-task Sub-task
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.1.0Release
  • Fix Version/s: 2.1.2Release
  • Component/s: Editor
  • Labels:
    None
  • Number of attachments :
    4

Description

When the quick fix dialog is shown, there should be some better ordering for potential fixes. This is particularly the case for the add import quick fix, which may have many types to propose.

Take a look at the class org.codehaus.groovy.eclipse.codeassist.proposals.Relevance, which calculates relevance for content assist. This may or may not be useful for you. In particular, you can look at how relevance is calculated for type proposals, in org.codehaus.groovy.eclipse.codeassist.processors.GroovyProposalTypeSearchRequestor.proposeImportableType()

It would be reasonable to refactor this logic into a separate class so that it would be accessible for quickfixes. If you go that way, then the logic from ...proposeNoImportType() and proposeConstructor() should be refactored out as well.

  • Options
    • Sort By Name
    • Sort By Date
    • Ascending
    • Descending
    • Download All

Attachments

  1. Text File
    fix_greclipse_936_minor_cleanup_v20110224_1341.patch
    24/Feb/11 12:46 PM
    12 kB
    Nieraj Singh
  2. Text File
    fix_greclipse_936_v20110219_2048.patch
    18/Feb/11 8:00 PM
    29 kB
    Nieraj Singh
  3. Text File
    fix_greclipse_936_v20110221_1704.patch
    21/Feb/11 4:08 PM
    29 kB
    Nieraj Singh
  4. Text File
    fix_greclipse_936_v20110221_1938.patch
    21/Feb/11 6:41 PM
    30 kB
    Nieraj Singh

Activity

Ascending order - Click to sort in descending order
  • All
  • Comments
  • Work Log
  • History
  • Activity
Hide
Permalink
Nieraj Singh added a comment - 01/Feb/11 2:04 PM

Another reference for implementing relevance for quick fixes is in jdt:

org.eclipse.jdt.internal.codeassist.CompletionEngine.proposeType(..)

It contains additional relevance calculations, although not all may be applicable to Groovy.

Show
Nieraj Singh added a comment - 01/Feb/11 2:04 PM Another reference for implementing relevance for quick fixes is in jdt: org.eclipse.jdt.internal.codeassist.CompletionEngine.proposeType(..) It contains additional relevance calculations, although not all may be applicable to Groovy.
Hide
Permalink
Andrew Eisenberg added a comment - 01/Feb/11 3:45 PM

Some more things to think about:

  1. Prioritize java, javax, groovy, groovyx, and grails imports over others
  2. Prioritize source types over binary types
  3. Prioritize types that share package prefixes (however the sharing should be more than com.* or org.*).
Show
Andrew Eisenberg added a comment - 01/Feb/11 3:45 PM Some more things to think about: Prioritize java, javax, groovy, groovyx, and grails imports over others Prioritize source types over binary types Prioritize types that share package prefixes (however the sharing should be more than com.* or org.*).
Hide
Permalink
Nieraj Singh added a comment - 17/Feb/11 5:04 PM


Initial implementation of Relevance rules for determining Type relevance for add missing import quick fix. This is not the final version, and is only attached for code review and debugging.

Show
Nieraj Singh added a comment - 17/Feb/11 5:04 PM Initial implementation of Relevance rules for determining Type relevance for add missing import quick fix. This is not the final version, and is only attached for code review and debugging.
Hide
Permalink
Andrew Eisenberg added a comment - 17/Feb/11 5:54 PM

Thanks for the patch. Appears to be working so far. I won't close this until I commit.

Show
Andrew Eisenberg added a comment - 17/Feb/11 5:54 PM Thanks for the patch. Appears to be working so far. I won't close this until I commit.
Hide
Permalink
Nieraj Singh added a comment - 17/Feb/11 8:28 PM

Attached new implementation with the following changes:

  • Different relevance rule implementation, where a rule is mapped to an enum rule type.
  • When constructing a RelevanceRules object, if no subset of rule types are specified (i.e., null or empty list of rule types), by default all the available rule types will be used when computing the relevance of a Java type.
  • Since the Accessibility rule has not been tested, it has been commented out from the add imports quick fix. The code for accessibility rule is still present in RelevanceRules, except that the subset of rules used by the quick fix excludes the accessibility rule.
  • Added missing javadocs and copyrights
Show
Nieraj Singh added a comment - 17/Feb/11 8:28 PM Attached new implementation with the following changes: Different relevance rule implementation, where a rule is mapped to an enum rule type. When constructing a RelevanceRules object, if no subset of rule types are specified (i.e., null or empty list of rule types), by default all the available rule types will be used when computing the relevance of a Java type. Since the Accessibility rule has not been tested, it has been commented out from the add imports quick fix. The code for accessibility rule is still present in RelevanceRules, except that the subset of rules used by the quick fix excludes the accessibility rule. Added missing javadocs and copyrights
Hide
Permalink
Nieraj Singh added a comment - 18/Feb/11 8:00 PM

Several changes from the last patch:

  • New rule: prioritising based on java, groovy, groovyx, javax and other libraries, in that order.
  • New rule: importing types from packages with similar package segments have higher priority, therefore importing com.test.TypeA into com.test.test.TypeB has higher priority than importing org.test.TypeA.
  • Changed the relevance API to accept a list of context types obtained from the context compilation unit. This allows additional rules that require comparison between the context project and the suggest import type project, as well as the rules mentioned above.
  • As some rules may clash with one another, Relevance rules now support Relevance categories, therefore each rule can define a Relevance category. Categories are defined by the existing: org.codehaus.groovy.eclipse.codeassist.proposals.Relevance.
  • Added more javadocs, cleaned up code, and fixed some bugs.
  • There are now 5 relevance rules: Source/Binary rule, type modifiers rule, type accessibility rule, java/groovy/groovyx/javax ordering rule, and similar package name rule.
Show
Nieraj Singh added a comment - 18/Feb/11 8:00 PM Several changes from the last patch: New rule: prioritising based on java, groovy, groovyx, javax and other libraries, in that order. New rule: importing types from packages with similar package segments have higher priority, therefore importing com.test.TypeA into com.test.test.TypeB has higher priority than importing org.test.TypeA. Changed the relevance API to accept a list of context types obtained from the context compilation unit. This allows additional rules that require comparison between the context project and the suggest import type project, as well as the rules mentioned above. As some rules may clash with one another, Relevance rules now support Relevance categories, therefore each rule can define a Relevance category. Categories are defined by the existing: org.codehaus.groovy.eclipse.codeassist.proposals.Relevance. Added more javadocs, cleaned up code, and fixed some bugs. There are now 5 relevance rules: Source/Binary rule, type modifiers rule, type accessibility rule, java/groovy/groovyx/javax ordering rule, and similar package name rule.
Hide
Permalink
Nieraj Singh added a comment - 21/Feb/11 4:08 PM

Changed relevance categories so that it can be used in content assist. The rule categories still may need to be adjusted, in particular the modifier rule.

Show
Nieraj Singh added a comment - 21/Feb/11 4:08 PM Changed relevance categories so that it can be used in content assist. The rule categories still may need to be adjusted, in particular the modifier rule.
Hide
Permalink
Nieraj Singh added a comment - 21/Feb/11 6:41 PM


Final tested patch. Commented out the Accessibility rule for this version of Groovy Eclipse as it is not fully tested. The remaining 4 rules have been tested. Also added logic to handle cases where imports are being suggested from within the same compilation unit, to make it compatible with content assist.

Show
Nieraj Singh added a comment - 21/Feb/11 6:41 PM Final tested patch. Commented out the Accessibility rule for this version of Groovy Eclipse as it is not fully tested. The remaining 4 rules have been tested. Also added logic to handle cases where imports are being suggested from within the same compilation unit, to make it compatible with content assist.
Hide
Permalink
Andrew Eisenberg added a comment - 22/Feb/11 1:24 PM

Thanks for the patch, Nieraj. I have committed it with some changes. I had originally hoped to use your patch largely as-is for type completion proposal ordering, but unfortunately, I had to make some fairly significant changes.

With quickfixes, you have access to the IType that you are proposing, but during content assist, you do not. There is only the fully qualified name, modifiers, and accessibility available. This means that rules like source/binary and same project cannot be calculated during content assist. Also, this means that I had to add a second method to IRelevanceRule that is applicable during content assist.

The result is something that is less clean than your original patch, but it is working in both situations. If you want to clean things up, then feel free to, but I think what we have now is sufficient.

Show
Andrew Eisenberg added a comment - 22/Feb/11 1:24 PM Thanks for the patch, Nieraj. I have committed it with some changes. I had originally hoped to use your patch largely as-is for type completion proposal ordering, but unfortunately, I had to make some fairly significant changes. With quickfixes, you have access to the IType that you are proposing, but during content assist, you do not. There is only the fully qualified name, modifiers, and accessibility available. This means that rules like source/binary and same project cannot be calculated during content assist. Also, this means that I had to add a second method to IRelevanceRule that is applicable during content assist. The result is something that is less clean than your original patch, but it is working in both situations. If you want to clean things up, then feel free to, but I think what we have now is sufficient.
Hide
Permalink
Nieraj Singh added a comment - 24/Feb/11 12:46 PM
  • Minor refactoring in the content assist support in RelevanceRules. For similarly named packages rule, the quick fix version should delegate to the content assist version without having to repeat the rule calculation. The fix simply removes the unnecessary calculation. All test scenarios pass.
Show
Nieraj Singh added a comment - 24/Feb/11 12:46 PM Minor refactoring in the content assist support in RelevanceRules. For similarly named packages rule, the quick fix version should delegate to the content assist version without having to repeat the rule calculation. The fix simply removes the unnecessary calculation. All test scenarios pass.
Hide
Permalink
Andrew Eisenberg added a comment - 24/Feb/11 1:39 PM

Thanks for the patch. It looks reasonable and I have applied it. I will be committing later today.

Show
Andrew Eisenberg added a comment - 24/Feb/11 1:39 PM Thanks for the patch. It looks reasonable and I have applied it. I will be committing later today.

People

  • Assignee:
    Nieraj Singh
    Reporter:
    Andrew Eisenberg
Vote (0)
Watch (0)

Dates

  • Created:
    15/Dec/10 11:56 AM
    Updated:
    24/Feb/11 1:39 PM
    Resolved:
    22/Feb/11 1:24 PM
  • Atlassian JIRA (v5.2.7#850-sha1:b2af0c8)
  • Report a problem
  • Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.