Bug 321564

Comment 1 Markus Keller 2011-04-26 05:20:51 EDT

We'll decide about this in RC1.

Comment 2 Markus Keller 2011-05-12 13:20:22 EDT

Created attachment 195515 [details]
Fix 1

The problem is that org.eclipse.ui.dialogs.SearchPattern is quite broken and FilteredItemsSelectionDialog already has too many hacks to just get it working. This whole thing needs to be deprecated and rewritten with a sane interface and a sane specification.

E.g. "A.t<" is considered as a camel case pattern, where the "<" only constrains the number of camel case tokens. OTOH, "a.t<" is an exact pattern, and since the "<" gets dropped somewhere in the implementation, we go and split base name and extension. The patch fixes this problem and thus solves 1).

Fixing 2 and 3 would involve a bigger rewrite of the CamelCase story.


P.S.: To create the files, run this:
echo "File contents: A.t" > A.t
echo "File contents: A.text" > A.text
echo "File contents: A.txt" > A.txt
echo "File contents: Alfie.txt" > Alfie.txt
echo "File contents: r.txt" > r.txt
echo "File contents: read.ext" > read.ext
echo "File contents: read.txt" > read.txt
echo "File contents: reada" > reada
echo "File contents: reada.eat" > reada.eat
echo "File contents: reada.ext" > reada.ext
echo "File contents: readaction" > readaction

Comment 3 Markus Keller 2011-05-12 13:21:03 EDT

Dani, I would release this for RC2.

Comment 4 Dani Megert 2011-05-16 11:07:35 EDT

The patch looks good to me. It fixes the problem for lowercase names and improves the fix that we made at the same location for uppercase names (see bug 323539).

+1 for RC2.

Comment 5 Dani Megert 2011-05-16 11:10:48 EDT

Oleg, could you review it as well? Thanks.

Comment 6 Oleg Besedin 2011-05-17 14:18:30 EDT

> 
> 1) Enter "read.ext<" into the dialog
> ==> also matches reada.ext.
> 
> 1 is a bug 

I am not sure that is the case. From what I understand, the purpose of "<" is to fix the number of camel-case entries, from the doc:

"terminating "<" or " " (space) to fix the number of camel-case parts, e.g. "CreS<" matches "CreateStuff.java", but not "CreateStuffNow.jsp" nor "Createstuff.txt". "

Because the resource dialog [on purpose] treats file name and extension as two separate filters, the (1) works as I would have expected it.

That said, I totally agree that the usage of {"<" / trailing space} in this dialog produces results that could be puzzling, even if well intended. It feels like a more complete fix after 3.7 is released might be a better way.

Comment 7 Markus Keller 2011-05-18 11:40:21 EDT

(In reply to comment #6)
I agree that the handling of "<" and " " is not optimal, especially in connection with camel case matches, where the terminators change the meaning to fix the number of camel case hunks. This conflicts with normal matches if the file name starts with an uppercase letter.

However, in the Open Resource dialog, camel case matching is only applied if the first character is uppercase. Case (1) is about patterns that start with a lowercase character. In that case, the fix from bug 319852 unfortunately doesn't work because SearchPattern#initializePatternAndMatchRule(String) cuts the terminator away in this case.

Note that the terminator worked fine in 3.5 but was completely broken in 3.6 for lowercase patterns. Camel case is out of the game here. Case (1) is just collateral damage of the name/extension separation that was added in 3.6 and that we already improved for 3.6.1 for the camel case patterns.

The addition of "&& getMatchRule() != SearchPattern.RULE_EXACT_MATCH" only restores the 3.5 behavior (to the same code path that is still used today when the pattern doesn't contain a ".").

We can have a discussion about camel case and terminators in 3.8, but that is really not related to the lowercase patterns problem.

Comment 8 Oleg Besedin 2011-05-18 11:48:29 EDT

I see, thanks for the explanation! +1 to the change.

Comment 9 Markus Keller 2011-05-18 11:50:23 EDT

Thanks, fixed in HEAD.

Comment 10 Dani Megert 2011-05-19 10:55:37 EDT

Verified in I20110519-0800.