[icedtea-web] RFC: Clean up some code in ClasspathMatcher
Jiri Vanek
jvanek at redhat.com
Wed Apr 2 16:53:49 UTC 2014
On 04/02/2014 06:17 PM, Omair Majid wrote:
> * 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.
If there are no wildcards, thenit is quoted as follows.,
return "^" + Pattern.quote(s) + "$";
The result IS java regexWhat am I missing?
>
> 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:
Oh yah, to much private for Andrew:( Sorry!
>
> 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.
Well thats crushing :)
Ok then with comment. Well do as you wish with the name to.
Ok to push in any of three suggested forms.
J.
More information about the distro-pkg-dev
mailing list