git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luke-Jr" <luke@dashjr.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Luke Dashjr <luke-jr+git@utopios.org>, git@vger.kernel.org
Subject: Re: [PATCH 1/5] port --ignore-unmatch to "git add"
Date: Thu, 13 Aug 2009 15:40:47 -0500	[thread overview]
Message-ID: <200908131540.49701.luke@dashjr.org> (raw)
In-Reply-To: <7vy6pna4lu.fsf@alter.siamese.dyndns.org>

On Thursday 13 August 2009 02:36:13 pm Junio C Hamano wrote:
> Luke Dashjr <luke-jr+git@utopios.org> writes:
> > "git rm" has a --ignore-unmatch option that is also applicable to "git
> > add" and may be useful for persons wanting to ignore unmatched arguments,
> > but not all errors.
>
> Chould you refresh my memory a bit?
>
> In what circumstance is "rm --ignore-unmatch" useful to begin with?
> A similar question for "add --ignore-unmatch".

Not sure on its purpose for "rm", but for "add"...
Avoiding a race condition in automation. In particular, if the file is deleted 
between the time the argument list is built until git scans for matches.

> Now the obligatory design level question is behind us, let's take a brief
> look at the codde.
>
> > +static int ignore_unmatch = 0;
>
> Drop " = 0" and let the language initialize this to zero.

Does C define a default initialisation of zero? My understanding was that 
uninitialised variables are always undefined until explicitly assigned a 
value.

> >  static void fill_pathspec_matches(const char **pathspec, char *seen, int
> > specs) {
> > @@ -63,7 +64,7 @@ static void prune_directory(struct dir_struct *dir,
> > const char **pathspec, int p fill_pathspec_matches(pathspec, seen,
> > specs);
> >
> >  	for (i = 0; i < specs; i++) {
> > -		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
> > +		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]) &&
> > !ignore_unmatch) die("pathspec '%s' did not match any files",
> >  					pathspec[i]);
> >  	}
> > @@ -108,7 +109,7 @@ static void refresh(int verbose, const char
> > **pathspec) refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED :
> > REFRESH_QUIET, pathspec, seen);
> >  	for (i = 0; i < specs; i++) {
> > -		if (!seen[i])
> > +		if (!seen[i] && !ignore_unmatch)
> >  			die("pathspec '%s' did not match any files", pathspec[i]);
> >  	}
> >          free(seen);
>
> What's the point of these two loops if under ignore_unmatch everything
> becomes no-op?
>
> That is, wouldn't it be much more clear if you wrote like this?

I'm not overly familiar with the git codebase, but wouldn't a null 'seen' 
variable break the refresh_index call? The loops themselves can be avoided, I 
suppose. I'll submit a new patch to optimise the changes (and rebase)...

  reply	other threads:[~2009-08-13 20:49 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-12 22:26 For review: git add --ignore-unmatch Luke-Jr
2009-08-12 22:57 ` Thomas Rast
2009-08-13  3:20 ` [PATCH 1/5] port --ignore-unmatch to "git add" Luke Dashjr
2009-08-13  3:20   ` [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors Luke Dashjr
2009-08-13  3:20     ` [PATCH 3/5] Document --ignore-unmatch in git-add.txt Luke Dashjr
2009-08-13  3:20       ` [PATCH 4/5] implement error_errno and warning_errno Luke Dashjr
2009-08-13  3:20         ` [PATCH 5/5] Convert add_file_to_index's lstat failure from a die to an error Luke Dashjr
2009-08-13 19:38     ` [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors Junio C Hamano
2009-08-13 20:42       ` Luke-Jr
2009-08-13 21:02       ` [PATCH] port --ignore-unmatch to "git add" Luke Dashjr
2009-08-13 21:03       ` [PATCH] Document --ignore-unmatch in git-add.txt Luke Dashjr
2009-08-13 19:36   ` [PATCH 1/5] port --ignore-unmatch to "git add" Junio C Hamano
2009-08-13 20:40     ` Luke-Jr [this message]
2009-08-13 21:51       ` Alex Riesen
2009-08-13 21:06     ` Thomas Rast
2009-08-14 19:52       ` Junio C Hamano
2009-08-14 20:39         ` Luke-Jr
2009-08-14 20:47           ` Luke-Jr
2009-08-14 21:39           ` Nanako Shiraishi
2009-08-14 22:54             ` Luke-Jr
2009-10-10 17:23               ` Luke-Jr

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=200908131540.49701.luke@dashjr.org \
    --to=luke@dashjr.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=luke-jr+git@utopios.org \
    /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).