[icedtea-web] RFC: Clean up some code in ClasspathMatcher

Omair Majid omajid at redhat.com
Wed Apr 2 16:17:20 UTC 2014


* Jiri Vanek <jvanek at redhat.com> [2014-04-02 11:02]:
> On 04/02/2014 04:51 PM, Omair Majid wrote:
> >The attached patch renames a method in ClasspathMatcher to better
> >reflect what the method does.
> 
> If you really insists - then converToRegEx would be even more
> suitable, or not? The method is not quoting or just chnaging
> wildcards * to .*, It is doing both :)

How about convertWildcardPatternToRegEx? I want to make it clear that
if you pass a non-wildcard pattern to this function, you are on your
own.

The conversion of '*' to equivalent regex and quoting everything else is
part of the overall conversion. It can't convert without quoting, but
how it quotes is an implementation detail.

> Also why to remove the comment? Andrew asked me for it explicitly
> when I wrote those peaces. So I think more people may be confused in
> future. Maybe at least add link to pattern documentation?

Let's break the comment down line-by-line.

    * coment for lazybones:

There is a typo here, and it calls the reader lazy. It doesn't provide
any information. There is not benefit of keeping this. So this line can
be safely removed.

    *  Pattern.quote - all characters in citation are threated as are without any special meaning

Again, typo. I don't understand 'citation' either. Maybe you meant
'escape' or 'quote'? But Pattern.quote is pretty much as explanatory as
'escape'.

The comment  also describes the description of the Pattern.quote method.
The javadocs of Pattern.quote do a better job describing what it does.

    *   Citation is based on \Q and \E marks wwith escapped inner \Q and \E

This line talks about \Q and \E. This is history and doesn't reflect the
current code at all. The history is stored in version control and I
don't see the point of adding historical context that isn't really
relevant any more to the current code.

    * ^ is start of th e line
    * $ is end of the line

These lines describe some of the simplest the regular expression
characters. Wouldn't be safe to assume the developer knows what a
regular expression is, or failing that, can look it up? Does it make
sense to document all the other regular expressions we are using in
icedtea-web too?

IMHO, the comment is adding no benefit at all.

Thanks,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681


More information about the distro-pkg-dev mailing list