Nick-The-Uncharted / tablefilter-swing

Automatically exported from code.google.com/p/tablefilter-swing
1 stars 1 forks source link

CustomChoice not working with wildcards #22

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. create a CustomChoice set having wildcards (e.g. "*text")
2. set a filter column to use the CustomChoice set

What is the expected output? What do you see instead?
I expect to see the custom choice in the filter editor combo, and selecting the 
item I expect to see all rows matching the wildcards. Instead, either the 
choice is not presented at all (indicating that the filter thought that there 
is no match), or when selecting the choice I only get part of the results. 

What version of the product are you using? On what operating system?
4.2.0; JVM 1.6.0u17 on WinXP

Please provide any additional information below.
It should be noted that when I type the expression directly in the filter 
combo's editbox ("*text") I get the the expected rows without any problem.

Original issue reported on code.google.com by altm...@gmail.com on 12 Dec 2011 at 2:21

GoogleCodeExporter commented 9 years ago
Hi, could you please send the code of your CustomChoice? 

The exact behaviour of a CustomChoice depends in fact of your implementation of 
the getFilter() method. The representation (*text) is only used to display the 
choice, has no impact on the produced filter.

Regards,

  Lu.

Original comment by coderazzi on 17 Dec 2011 at 9:04

GoogleCodeExporter commented 9 years ago
The code is very simple. I do not use custom Java classes but rather use the 
defaults, as follows:

   cc1 = net.coderazzi.filters.gui.CustomChoice.create('*Lo','Long');
   cc2 = net.coderazzi.filters.gui.CustomChoice.create('*Sh','Short');
   choices = java.util.HashSet();
   choices.add(cc1); choices.add(cc2);
   filter = net.coderazzi.filters.gui.TableFilterHeader(jtable);
   filter.getFilterEditor(colIdx).setCustomChoices(choices);

This fails to discover cell contents such as "Long" or "<html><span 
style="background-color:#00ff00;">Long..." - either the choice is not presented 
at all (indicating that the filter thought that there is no match), or when 
selecting the choice I only get part of the results. 

My workaround was to specify CustomChoices with the exact data contents (which 
works
as expected), but I would obviously like wildcards to work.

Original comment by altm...@gmail.com on 17 Dec 2011 at 7:54

GoogleCodeExporter commented 9 years ago
Hi,

the way CREATE works, it expects a full match (ignoring case eventually), no 
regular expressions involved here. It is obviously easy to extend this 
behavior, but it would penalize the simpler cases where an exact match is 
required. I will see how this can be better accomplished (including an 
additional CREATE method does not seem the best option).

Best regards,

  Lu.

Original comment by coderazzi on 19 Dec 2011 at 12:45

GoogleCodeExporter commented 9 years ago
How about an extra optional input param flag (default=false) indicating whether 
the new CustomChoice is a regex?

Original comment by altm...@gmail.com on 19 Dec 2011 at 1:08

GoogleCodeExporter commented 9 years ago
Hi; implementing the optional parameter, with Java not directly supporting 
them, implies still an additional method.... which perhaps is not needed. My 
point is that I was in fact blindfolded, not remembering why I hadn't included 
wildcards support from the first moment.

The usage of wildcards on the filters depends on the parser implementation 
(net.coderazzi.filters.IParser). The default one(the only one supplied with the 
library) implements the pseudo regular expressions support, and is therefore 
responsible to filter, for the expression "*Long*", all column's values 
containing the string "Long".

The CustomChoice has no information on the parser used (columns with rendered 
content lack a parser, for once), so there are no solid grounds to interpret 
the string as a regular expression. A programmer could change the parser 
implementation to use SQL-like wildcards (% instead of *), and this would break 
the suggested behaviour for CustomChoice.

So, I could extend the interfaces to relate the CustomChoice with the IParser, 
but I think a better solution is to accept a java.util.Pattern instance as 
CustomChoice.create parameter. In this case, your initial example would simply 
be: 

cc1 = 
net.coderazzi.filters.gui.CustomChoice.create(Pattern.compile('.*Lo'),'Long');

More verbose, but seems also more generic, what do you think?

Best regards,

 Luis

Original comment by coderazzi on 20 Dec 2011 at 3:02

GoogleCodeExporter commented 9 years ago
I agree - this way the interface of create(Object) would not need to be 
modified, only the internal implementation, and similarly for 
createSet(Object[] or Collection).

I would suggest modifying the constructor methods similarly, to accept Object 
rather than String as the first input arg, so that they could also support 
Pattern.

As a final modification, I suggest adding the missing public methods 
setIcon(Icon) and setPrecedence(int) - the CustomChoice class already includes 
the getter methods for them, and allows to set them using the constructors, but 
there are no setter methods so users like me who use the static create() 
methods have no way to set the icon and precedence. 

Original comment by altm...@gmail.com on 20 Dec 2011 at 7:29

GoogleCodeExporter commented 9 years ago
Hi, done this feature, I included the setIcon and setPrecedence methods, plus 
the inclusion of the Pattern on the create method.

The createSet method does not benefit much from this change, as the default 
representation would be the string associated to the Pattern -normally, not 
very user friendly-. For this reason, I have included also setRepresentation 
and getRepresentation methods.

For your suggestion about the constructor, please note that the string provided 
as parameter is just the representation of the custom choice, must remain as 
String.

I have commited the changes into the mercurial repository, I plan to deliver it 
together with issue 23.

Original comment by coderazzi on 20 Dec 2011 at 11:52

GoogleCodeExporter commented 9 years ago
thanks Luis

Regarding the constructor - my mistake: I thought that the input parameter is 
the pattern not the representation. But in this case, I think it makes sense to 
add setPattern and getPattern methods so that users could modify the pattern 
just like they can now modify the representation, icon and precedence.

Regarding createSet, I think that a createSet variant that also accepts a 
java.util.Map will make it easier for users to create a set of CustomChoices 
with pattern-representation pairs, instead of having to use the new 
setRepresentation method for each CustomChoice separately. I think that it 
would make external code simpler and more maintainable. Admittedly, this is not 
a major issue.

Sorry if all this is turning CustomChoice into a bloated monster... :-)

Original comment by altm...@gmail.com on 21 Dec 2011 at 12:23

GoogleCodeExporter commented 9 years ago
Indeed it would become really bloated! I do not know, these are helper methods 
to perform a precise task, I am more tempted to deprecate createSet and remove 
it in the future. If a user ever needs to use it, he/she could implement it 
easily. Better than providing infinite methods trying to cover all small 
nuances (as I am sure that new scenarios would appear).

Concerning setPattern / getPattern, please note that a CustomChoice does not 
require a Pattern at all (see for example the custom choices 
CustomChoice.MATCH_ALL or CustomChoice.MATCH_EMPTY).

Cheers, and thanks for your feedback!

  Lu.

Original comment by coderazzi on 21 Dec 2011 at 1:28

GoogleCodeExporter commented 9 years ago
Fixed on release 4.3.0

Original comment by coderazzi on 16 Jan 2012 at 10:44