From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 29 May 2008 13:55:11 GMT
Subject: Re: [PATCH] make commit --interactive lock index
Message-ID: <alpine.DEB.1.00.0805291454270.13507@racer.site.net>
In-Reply-To: <483EABD8.3050600@gnu.org>

Hi,

On Thu, 29 May 2008, Paolo Bonzini wrote:

> Johannes Schindelin wrote:
> 
> > On Thu, 29 May 2008, Paolo Bonzini wrote:
> > 
> > > @@ -233,6 +228,8 @@ static char *prepare_index(int argc, const char
> > > **argv, const char *prefix)
> > >   if (*argv)
> > >    pathspec = get_pathspec(prefix, argv);
> > > 
> > > +	assert (!(interactive && pathspec && *pathspec));
> > 
> > As pathspec is specified indirectly by the user, I think an assert() 
> > here is actively wrong.
> 
> But the program may still guarantee a condition by checking it 
> elsewhere.  I don't need to teach you about that, do I?  In particular, 
> the assert checks that this:
> 
> if (interactive && argc > 0)
>         die("Paths with --interactive does not make sense.");
> 
> ... is equivalent to !pathspec || !*pathspec.

Okay, I have to spell it out:

I think that the assert() here is not helpful at all, and that you should 
rather do the "if () die()" thingie.

Ciao,
Dscho