Re: [PATCH] Reuse previous annotation when overwriting a tag
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Nov 3, 2007, 11:54 UTC
- Message-ID
- <Pine.LNX.4.64.0711031148460.4362@racer.site>
- In-Reply-To
- <1194082273-19486-1-git-send-email-mh@glandium.org>
Hi,
On Sat, 3 Nov 2007, Mike Hommey wrote:
Show 9 quoted lines
> diff --git a/builtin-tag.c b/builtin-tag.c > index 66e5a58..cfd8017 100644 > --- a/builtin-tag.c > +++ b/builtin-tag.c > @@ -247,9 +247,42 @@ static int git_tag_config(const char *var, const char *value) > return git_default_config(var, value); > } > > +static void write_annotation(int fd, const unsigned char *sha1)
Technically, it is the "body".
Show 11 quoted lines
> +{
> + int i;
> + unsigned long size;
> + enum object_type type;
> + char *buf, *sp, *eol;
> + size_t len;
> +
> + sp = buf = read_sha1_file(sha1, &type, &size);
> + if (!buf)
> + return;
> + if (!size || (type != OBJ_TAG)) {Please lose the extra parents.
Show 7 quoted lines
> + free(buf); > + return; > + } > + /* skip header */ > + while (sp + 1 < buf + size && > + !(sp[0] == '\n' && sp[1] == '\n')) > + sp++;
This can be done much easier with 'sp = strstr(buf, "\n\n");'. You can even do that before the previous if(), to free() && return if there is no body.
Show 12 quoted lines
> + /* strip the signature */
> + for (i = 0, sp += 2; sp < buf + size &&
> + prefixcmp(sp, PGP_SIGNATURE "\n");
> + i++) {
> + eol = memchr(sp, '\n', size - (sp - buf));
> + len = eol ? eol - sp : size - (sp - buf);
> + write_or_die(fd, sp, len + 1);
> + if (!eol)
> + break;
> + sp = eol + 1;
> + }
> + free(buf);This can be done much easier with 'eob = strstr(sp, "\n" PGP_SIGNATURE "\n");'.
Show 6 quoted lines
> +} > + > static void create_tag(const unsigned char *object, const char *tag, > struct strbuf *buf, int message, int sign, > - unsigned char *result) > + unsigned char *prev, unsigned char *result)
This changes indentation.
Show 9 quoted lines
> @@ -282,6 +315,10 @@ static void create_tag(const unsigned char *object, const char *tag,
> if (fd < 0)
> die("could not create file '%s': %s",
> path, strerror(errno));
> +
> + if (prev)
> + write_annotation(fd, prev);
> +
> write_or_die(fd, tag_template, strlen(tag_template));Isn't an "else" missing before the write_or_die() here?
Show 21 quoted lines
> @@ -308,7 +345,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> {
> struct strbuf buf;
> unsigned char object[20], prev[20];
> - int annotate = 0, sign = 0, force = 0, lines = 0, message = 0;
> + int annotate = 0, sign = 0, force = 0, lines = 0,
> + message = 0, existed = 0;
> char ref[PATH_MAX];
> const char *object_ref, *tag;
> int i;
> @@ -417,9 +455,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> hashclr(prev);
> else if (!force)
> die("tag '%s' already exists", tag);
> + else
> + existed = 1;
>
> if (annotate)
> - create_tag(object, tag, &buf, message, sign, object);
> + create_tag(object, tag, &buf, message, sign,
> + existed ? prev : NULL, object);Why not teach write_annotations() (or write_tag_body() like I would prefer it to be called) to grok a null_sha1? It's not like we care for performance here, but rather for readability and ease of use.
Ciao, Dscho