search

coderazzi / tablefilter-swing

TableFilter is a set of Swing components to support user-driven filtering on table.
https://coderazzi.net/tablefilter
MIT License
7 stars 2 forks source link

CustomChoice not working with wildcards #22

Closed coderazzi closed 12 years ago

coderazzi commented 12 years ago

Original report by altmany (Bitbucket: altmany, GitHub: altmany).

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 altmany on 2011-12-12 14:21:15

coderazzi commented 12 years ago

Original comment by coderazzi coderazzi (Bitbucket: coderazzi, GitHub: coderazzi).

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 issue reported on code.google.com by coderazzi on 2011-12-17 09:04:55

coderazzi commented 12 years ago

Original comment by altmany (Bitbucket: altmany, GitHub: altmany).

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 issue reported on code.google.com by altmany on 2011-12-17 19:54:07

coderazzi commented 12 years ago

Original comment by coderazzi coderazzi (Bitbucket: coderazzi, GitHub: coderazzi).

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 issue reported on code.google.com by coderazzi on 2011-12-19 00:45:24

coderazzi commented 12 years ago

Original comment by altmany (Bitbucket: altmany, GitHub: altmany).

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

Original issue reported on code.google.com by altmany on 2011-12-19 01:08:26

coderazzi commented 12 years ago

Original comment by coderazzi coderazzi (Bitbucket: coderazzi, GitHub: coderazzi).

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 issue reported on code.google.com by coderazzi on 2011-12-20 03:02:20

coderazzi commented 12 years ago

Original comment by altmany (Bitbucket: altmany, GitHub: altmany).

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 issue reported on code.google.com by altmany on 2011-12-20 07:29:32

coderazzi commented 12 years ago

Original comment by coderazzi coderazzi (Bitbucket: coderazzi, GitHub: coderazzi).

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 issue reported on code.google.com by coderazzi on 2011-12-20 23:52:40

coderazzi commented 12 years ago

Original comment by altmany (Bitbucket: altmany, GitHub: altmany).

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 issue reported on code.google.com by altmany on 2011-12-21 00:23:19

coderazzi commented 12 years ago

Original comment by coderazzi coderazzi (Bitbucket: coderazzi, GitHub: coderazzi).

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 issue reported on code.google.com by coderazzi on 2011-12-21 01:28:15

coderazzi commented 12 years ago

Original comment by coderazzi coderazzi (Bitbucket: coderazzi, GitHub: coderazzi).

Fixed on release 4.3.0

Original issue reported on code.google.com by coderazzi on 2012-01-16 22:44:30