git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, johannes@familieschneider.info
Subject: Re: [PATCH 3/3] implement pattern matching in ce_path_match
Date: Wed, 14 Jan 2009 20:23:41 +0100	[thread overview]
Message-ID: <20090114192341.GA26703@localhost> (raw)
In-Reply-To: <7vljtd20m6.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 14, 2009 at 10:39:29AM -0800, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
> 
> > With this patch ce_path_match uses match_pathspec in order to perform
> > pattern matching.
> 
> We have two conflicting definitions of pattern matching in our system.
> I'd make it more explicit which kind of pattern matching you are talking
> about here.

Right, will fix.

> In the longer term we really should unify them by teaching the former to
> fall back to globbing without getting undue performance hit, and this
> patch may be a step in the right direction.  There are optimizations that
> assume the "leading path" semantics to trim the input early and avoid
> opening and descending into a tree object if pathspec patterns cannot
> possibly match (see tree-diff.c::tree_entry_interesting() for an example),
> and we need to teach them to notice a glob wildcard in an earlier part of
> a pathspec and to descend into some trees that they would have skipped
> with the old definition of pathspec.

I see. I can probably fix that this weekend.

> > @@ -49,7 +60,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
> >  		rev->always_show_header = 0;
> >  	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
> >  		rev->always_show_header = 0;
> > -		if (rev->diffopt.nr_paths != 1)
> > +		if (rev->diffopt.nr_paths != 1 || has_special(rev->diffopt.paths[0]))
> >  			usage("git logs can only follow renames on one pathname at a time");
> >  	}
> 
> The reason match_pathspec() first tries exact match and then falls back to
> globbing is so that the user can say "I have a file whose name ends with a
> question mark, please match it literally."  This patch defeats it, but it
> probably is a minor point.

I was wondering actually if we should disallow such paths altogether,
since there would be no way to match only 'a?', if something like 'ab' also
exists. So if you added 'a?' by accident, you cannot even remove it without
also removing 'ab'.

I think we could at least add an option to disable globbing. Then we can
also disable the above check conditioned on that. If we allowed globbing
pattern for following renames wouldn't that result in following the first
file (or last in history) to match the pattern, which is potentially
confusing?

  reply	other threads:[~2009-01-14 19:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-14 14:54 [PATCH 0/3] fix "git add" pattern matching Clemens Buchacher
2009-01-14 14:54 ` [PATCH 1/3] clean up pathspec matching Clemens Buchacher
2009-01-14 14:54   ` [PATCH 2/3] remove pathspec_match, use match_pathspec instead Clemens Buchacher
2009-01-14 14:54     ` [PATCH 3/3] implement pattern matching in ce_path_match Clemens Buchacher
2009-01-14 15:25       ` Clemens Buchacher
2009-01-14 15:44       ` Johannes Schindelin
2009-01-14 15:55         ` Sverre Rabbelier
2009-01-14 16:18         ` Samuel Tardieu
2009-01-14 16:53           ` Jeff King
2009-01-14 18:39       ` Junio C Hamano
2009-01-14 19:23         ` Clemens Buchacher [this message]
2009-01-14 22:27           ` Junio C Hamano
2009-01-15  8:20             ` Clemens Buchacher
2009-01-16  2:51         ` Junio C Hamano
2009-01-14 15:40     ` [PATCH 2/3] remove pathspec_match, use match_pathspec instead Johannes Schindelin
2009-01-14 16:09       ` Clemens Buchacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090114192341.GA26703@localhost \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes@familieschneider.info \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).