Hi,
On Wed, 28 Nov 2007, Alex Riesen wrote:
> Johannes Schindelin, Wed, Nov 28, 2007 13:18:10 +0100:
> > On Tue, 27 Nov 2007, Alex Riesen wrote:
> >
> > > Could not stop myself. Hopefully didn't beat anyone to it :)
> > > Almost all code shamelessly stolen from builtin-diff-index.c.
> >
> > Then I have to wonder if it would not be a better idea to refactor the
> > code, so that other people do not have to steal the code again, but are
> > able to reuse it ;-)
>
> Not sure it will be worth the effort. It is really short.
Okay.
> > > Preprocessor trickery in DIFF_OPT_* macros is disgusting, it breaks Vim
> > > word completion and trying to use many flags in one expression looks
> > > just ugly.
> >
> > How does it break Vim word completion? And why should something like
> >
> > DIFF_OPT_SET(&rev.diffopt, QUIET | EXIT_WITH_STATUS);
> >
> > look ugly? I find it highly readable.
>
> Oh, this would look ok. It just wont compile: DIFF_OPT_SET prepends
> second argument with DIFF_OPT_:
>
> #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
> #define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
> #define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)
Ouch. Sorry for suggesting it when I clearly had no clue.
> > > + if (no_edit) {
> > > + static const char *argv[] = { NULL, "HEAD", NULL };
> > > + struct rev_info rev;
> > > + unsigned char sha1[40];
> > > + int is_initial;
> > > +
> > > + fclose(fp);
> > > +
> > > + if (!active_nr && read_cache() < 0)
> > > + die("Cannot read index");
> > > +
> > > + if (get_sha1("HEAD", sha1) != 0)
> > > + return !!active_nr;
> >
> > Don't want to be anal here, but are there possibly reasons (read "possible
> > errors") other than an empty repo where this triggers?
>
> Definitely. I just don't know. OTOH, I can only return "committable" or
> "not committable".
I guess it is good enough. Just wanted to point out that this can fail if
.git/HEAD is not readable.
> > Besides, would this not be more elegant as
> >
> > setup_revisions(0, NULL, &rev, "HEAD");
>
> Hmm... And I was so puzzled as to what that "def" argument could
> possibly mean... Still am, in fact. But it works.
It is the mechanism which makes "git log" default to "git log HEAD".
> > Other than that (including my remark about refactoring that piece of
> > code), I like it.
>
> Me too: I have *extensively* tested it today and a commit on the
> 2.6GHz/2Gb/SATA windows machine is almost as fast as on my linux
> laptop now (Centrino/1.2GHz downclocked to 800MHz/384Mb/IDE).
Yes, I suspected something like this would happen.
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
