Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 144355

Summary:

[syntax highlighting] case insensitive WordRule

Product:

[Eclipse Project] Platform

Reporter:

Alex Le <unbonnevie>

Component:

Text

Assignee:

Platform-Text-Inbox <platform-text-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

enhancement

Priority:

CC:

caniszczyk, daniel_megert, wassim.melhem

Version:

3.2

Keywords:

helpwanted

Target Milestone:

3.3 M4

Hardware:

OS:

All

Whiteboard:

Bug Depends on:

Bug Blocks:

151172

Attachments:

Description	Flags
org.eclipse.jface.text.patch	none
org.eclipse.jface.text.path	none

Description Alex Le

2006-05-29 23:40:23 EDT

Hi,

The method WordRule.evaluate(ICharacterScanner scanner) needs to be flexible in checking if a word is in the map.   The checking can be done with case insensitive or not. This way, the user does not have to specify "XYZ", "xyz", "Xyz", etc., as the same word.

For example, we can add WordRule.setIgnoreCase(boolean).  If this is set, then the statement "IToken token= (IToken) fWords.get(fBuffer.toString());" could be "IToken token= (IToken) fWords.get(fBuffer.toString().toUpperCase());" where the user has specified all the words in upper case.

Comment 1 Tom Hofmann

2006-05-30 03:59:20 EDT

A case insensitive word rule makes sense for certain languages (e.g. SQL). However, there are currently no plans to implement this. Feel free to provide a high quality patch including tests.

Comment 2 Chris Aniszczyk

2006-11-16 16:50:13 EST

seems reasonable

Comment 3 Chris Aniszczyk

2006-11-16 16:51:06 EST

Created attachment 54028 [details]
org.eclipse.jface.text.patch

Here is a stab at the desired functionality. Included is a unit test. Note my in depth knowledge of jface text isn't great ;)

Comment 4 Chris Aniszczyk

2006-11-16 16:51:30 EST

cc'ng Wassim as this is of interest to PDE

Comment 5 Alex Le

2006-11-16 17:23:05 EST

(In reply to comment #4)
> cc'ng Wassim as this is of interest to PDE
> 

THANKS!

Comment 6 Wassim Melhem

2006-11-16 18:54:17 EST

Hi Dani,

This function is very important to the syntax highlighting of the MANIFEST.MF where the header names are case-insensitive.

If the patch looks good to you, please release or ask our friend Chris to revise, if necessary.  Thanks.

Comment 7 Dani Megert

2006-11-17 01:50:02 EST

Will look at this during M4.

Comment 8 Dani Megert

2006-11-20 05:50:01 EST

Hi Chris,

the patch has two potential problems:
1) when setIgnoreCase is called after words have been added. I suggest to only allow to set this in the constructor.

2) the Georgian alphabet isn't treated right with this patch, see comments in
   String.regionMatches(...). If we assume that the word list is not too big 
   then we could iterate over the entries and search for the first word that 
   matches using equalsIgnoreCase(...) instead of using get(...). We would only
   do this if ignoreCase is true.

Minor details:
- call the existing constructor instead of copying its code
- don't use 'this' qualification
- use compact assignment, e.g. "foo= value" instead of "foo = value"

Comment 9 Chris Aniszczyk

2006-11-20 12:40:04 EST

Created attachment 54182 [details]
org.eclipse.jface.text.path

Updated patch to comments mentioned and also made it adhere to Zurichian Notation :)

Comment 10 Dani Megert

2006-11-21 09:03:52 EST

>- call the existing constructor instead of copying its code
This has not been done.

>Updated patch to comments mentioned and also made it adhere to Zurichian
>Notation
Almost - there are still some ugly non-compact assignments in the patch ;-)

The loop continues even though the key has been found. This is bad for performance.


I've fixed the above things and also a typo and a missing @since tag on the new field.

Cmmitted the code to HEAD.
Available in builds > I20061121-0800.