Re: [PATCHv3 1/5] refs: add match_pattern()

On Wed, Feb 22, 2012 at 04:17:22PM -0800, Junio C Hamano wrote:

Tom Grennan [off-list ref] writes:

quoted

Yes, I tried to minimize change but the conversion of argv's to
string_list's won't add that much.

How about _not_ using string_list?  After all, string_list is not just a
collection of strings, but is a table to hold strings with attributes.  I
thought argv_array is more appropriate abstraction for the purpose of your
patch.

OK, It looks like I should also add a common parse_opt_argv_array() to
parse-options-cb.  Of course that would be in a separate, dependent
patch.

quoted

quoted

ref_match_pattern("refs/heads/master",
       		  ["maste?", NULL],
                         ["refs/heads/", NULL],
                         (REF_MATCH_FNM_PATH|REF_MATCH_LEADING));

Note that the above "ref_match_pattern()" gives the same "flags" for the
call to match_one() for elements in both positive and negative array and
it is very deliberate.  See review comment to [3/5] for the reasoning.

OK, I think that I understand, but please confirm, you'd expect no output in
the above example, right?

"maste?" would match with FNM_PATHNAME with "refs/heads/master" but
the negative "refs/heads/" matches with it, so yeah, I expect that the
function would return false.

thanks, that's a good test case.

-- 
TomG