Re: [PATCH] Port git commit to C.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



A huge diff.

I'll quote the difference between this version and the version
previously parked on 'pu', and comment, after re-reverting what
you reverted from the parked version (which has minor tweaks
such as removing git-runstatus from .gitignore I made
previously).

>  builtin-commit.c |   88 +++++++++++++++++++++++++++---------------------------
>  1 files changed, 44 insertions(+), 44 deletions(-)
> 
> diff --git a/builtin-commit.c b/builtin-commit.c
> index f108e90..669cc6b 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -70,25 +70,22 @@ static char *prepare_index(const char **files, const char *prefix)
>  	struct tree *tree;
>  	struct lock_file *next_index_lock;
>  
> +	if (interactive) {
> +		interactive_add();
> +		return get_index_file();
> +	}
> +

So interactive bypasses everything else, which makes sense.

What happens when the user aborts interactive_add, wishing to
abort the whole commit process?

> @@ -267,36 +264,41 @@ static int message_is_empty(struct strbuf *sb, int start)
>  
>  static void determine_author_info(struct strbuf *sb)
>  {
> -	char *p, *eol;
> -	char *name = NULL, *email = NULL;
> +	char *name, *email, *date;
>  
> -	if (force_author) {
> -		const char *eoname = strstr(force_author, " <");
> -		const char *eomail = strchr(force_author, '>');
> +	name = getenv("GIT_AUTHOR_NAME");
> +	email = getenv("GIT_AUTHOR_EMAIL");
> +	date = getenv("GIT_AUTHOR_DATE");
>  
> -		if (!eoname || !eomail)
> -			die("malformed --author parameter\n");
> -		name = xstrndup(force_author, eoname - force_author);
> -		email = xstrndup(eoname + 2, eomail - eoname - 2);
> -		/* REVIEW: drops author date from amended commit on --amend --author=<author> */
> -		strbuf_addf(sb, "author %s\n",
> -			    fmt_ident(name, email,
> -				      getenv("GIT_AUTHOR_DATE"), 1));
> -		free(name);
> -		free(email);
> -	} else if (use_message) {
> -		p = strstr(use_message_buffer, "\nauthor");
> -		if (!p)
> +	if (use_message) {
> +		const char *a, *lb, *rb, *eol;
> +
> +		a = strstr(use_message_buffer, "\nauthor ");
> +		if (!a)
>  			die("invalid commit: %s\n", use_message);

You know in what sense it is invalid here and ...

> -		p++;
> -		eol = strchr(p, '\n');
> -		if (!eol)
> +
> +		lb = strstr(a + 8, " <");
> +		rb = strstr(a + 8, "> ");
> +		eol = strchr(a + 8, '\n');
> +		if (!lb || !rb || !eol)
>  			die("invalid commit: %s\n", use_message);

... here.  Would it be more helpful to say "No author line", and
"Cannot parse author line", I wonder.  Probably too much.

> -		strbuf_add(sb, p, eol + 1 - p);
> -	} else {
> -		strbuf_addf(sb, "author %s\n", git_author_info(1));
> +		name = xstrndup(a + 8, lb - (a + 8));
> +		email = xstrndup(lb + 2, rb - (lb + 2));
> +		date = xstrndup(rb + 2, eol - (rb + 2));
>  	}
> +
> +	if (force_author) {
> +		const char *lb = strstr(force_author, " <");
> +		const char *rb = strchr(force_author, '>');
> +
> +		if (!lb || !rb)
> +			die("malformed --author parameter\n");
> +		name = xstrndup(force_author, lb - force_author);
> +		email = xstrndup(lb + 2, rb - (lb + 2));
> +	}
> +
> +	strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, 1));

I see a slight leak here of name/email/date depending on how
they are obtained.  Probably it is too much hassle to deal with
for too little gain.
-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux