Bug 483512 - [content assist] Substring completion breaks "Insert common prefixes automatically"
Summary: [content assist] Substring completion breaks "Insert common prefixes automati...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.6   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 4.6 M5   Edit
Assignee: Noopur Gupta CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-02 14:16 EST by Markus Keller CLA
Modified: 2020-06-16 03:12 EDT (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2015-12-02 14:16:16 EST 
I20151201-1100

When substring matches are enabled, the "Insert common prefixes automatically" feature is often broken.

Example (JDK 8):

        String s = "";
        s.ind

Without substring matching, content assist after "ind" completes to "indexOf". This doesn't work any more, because the "lastIndexOf" proposals use an uppercase "I". I would expect the common prefix of the prefix matches to be inserted (i.e. "indexOf").


The feature still works here:

        s.su

It completes to "sub" and proposes "subSequence" and "substring". This example should keep working as is. I.e. it should not auto-complete to "subS" or "subs".


I find it very confusing that "Insert common prefixes" auto-inserts "startsWith" here: (completing to "tartsWith" would be fine)

        s.tar

But it doesn't complete "Point" here:

        s.Po

And here, it even steals the last character the user entered: (produces "s.sub"!)

        s.ubs


Suggested fix:

- "Insert common prefixes" should never add or remove any character in front of the caret. (Changing the case is fine, e.g. for "s.Sub".)
   => Keeps the user's text stable

- If there is any prefix match, it should not consider the casing of the substring matches to compute the common prefix to insert.
   => Avoids the "ind" problem

- If there are only substring matches, it should insert the completion that is common to all substring matches
   => Completes common prefix of substrings if applicable
Comment 1 Eclipse Genie CLA 2015-12-14 09:56:26 EST 
New Gerrit change created: https://git.eclipse.org/r/62628
Comment 2 Eclipse Genie CLA 2015-12-14 09:56:49 EST 
New Gerrit change created: https://git.eclipse.org/r/62629
Comment 3 Dani Megert CLA 2016-01-12 10:43:39 EST 
I had an initial look at the patches. I don't like that they introduce the concept of substring matches in Platform Text where such a concept does not exist.

Again, I didn't look into this in detail, but isn't there an easier fix in JDT that just uses #indexOf to figure out the common prefix?
Comment 4 Noopur Gupta CLA 2016-01-12 12:37:11 EST 
(In reply to Dani Megert from comment #3)
> I had an initial look at the patches. I don't like that they introduce the
> concept of substring matches in Platform Text where such a concept does not
> exist.
> 
> Again, I didn't look into this in detail, but isn't there an easier fix in
> JDT that just uses #indexOf to figure out the common prefix?

While extracting the common prefix, it is required to know if the casing should be considered for prefix computation and that depends on the presence of substring proposals in the filtered list of proposals. 

The algorithm for extracting the common prefix from filtered list of proposals is implemented in the private method of non-API class: CompletionProposalPopup.completeCommonPrefix(), which also inserts that prefix in the document. 

I don't see an easier fix as all this is not accessible from JDT.
Comment 5 Dani Megert CLA 2016-01-13 08:54:10 EST 
(In reply to Noopur Gupta from comment #4)
> While extracting the common prefix, it is required to know if the casing
> should be considered for prefix computation and that depends on the presence
> of substring proposals in the filtered list of proposals. 

Actually that code triggered my comment: it assumes to know how the client (e.g. JDT) decided that it is a sub-string proposal. I was thinking of having that information already set on the proposal (via new API). However, after sleeping over it and discussing with Markus, this would be an overkill. But we definitely have to document why we add a new case for 'hasSubstringMatch'.