Re: [PATCH] branch as a builtin (again)

From: David Rientjes <rientjes@google.com>
Date: 2006-08-21 17:49:41
On Sun, 20 Aug 2006, Kristian Høgsberg wrote:

> diff --git a/builtin-branch.c b/builtin-branch.c
> new file mode 100644
> index 0000000..25c6313
> --- /dev/null
> +++ b/builtin-branch.c
> @@ -0,0 +1,170 @@
> +/*
> + * Builtin "git branch"
> + *
> + * Copyright (c) 2006 Kristian Høgsberg <krh@redhat.com>
> + * Based on git-branch.sh by Junio C Hamano.
> + */
> +
> +#include "cache.h"
> +#include "refs.h"
> +#include "commit.h"
> +#include "builtin.h"
> +
> +static const char builtin_branch_usage[] =
> +    "git-branch [(-d | -D) <branchname>] | [[-f] <branchname> [<start-point>]] | -r";
> +
> +
> +static int remote_only = 0;

Unnecessary initialization

> +static const char *head;
> +static unsigned char head_sha1[20];
> +
> +static int in_merge_bases(const unsigned char *sha1,
> +			  struct commit *rev1,
> +			  struct commit *rev2)
> +{
> +	struct commit_list *bases, *b;
> +
> +	bases = get_merge_bases(rev1, rev2, 1);
> +	for (b = bases; b != NULL; b = b->next) {
> +		if (!hashcmp(sha1, b->item->object.sha1)) {
> +			free_commit_list(bases);
> +			return 1;
> +		}
> +	}
> +
> +	free_commit_list(bases);
> +	return 0;

Make it cleaner for the future:
{
	int ret = 0;
	...
	for (b = bases; b; b = b->next) {
		if (!hashcmp(sha1, b->item->object.sha1)) {
			ret = 1;
			break;
		}
	}

	free_commit_list(bases);
	return ret;
}

> +}
> +
> +static void delete_branches(int argc, const char **argv, int force)
> +{
> +	struct commit *rev1, *rev2;
> +	unsigned char sha1[20];
> +	const char *p, *name;
> +	int i;
> +
> +	for (i = 0; i < argc; i++) {
> +		if (!strcmp(head, argv[i]))
> +			die("Cannot delete the branch you are currently on.");
> +
> +		name = git_path("refs/heads/%s", argv[i]);
> +		p = resolve_ref(name, sha1, 1);
> +		if (p == NULL)
> +			die("Branch '%s' not found.", argv[i]);

p is unnecessary:
	if (!resolve_ref(name, sha1, 1))
		die(...);

> +
> +		rev1 = lookup_commit_reference(sha1);
> +		rev2 = lookup_commit_reference(head_sha1);

Are these both needed inside every iteration?

> +		if (!rev1 || !rev2)
> +			die("Couldn't look up commit objects.");
> +
> +		/* This checks wether the merge bases of branch and
> +		 * HEAD contains branch -- which means that the HEAD
> +		 * contains everything in both.
> +		 */
> +
> +		if (!force &&
> +		    !in_merge_bases(sha1, rev1, rev2)) {
> +			fprintf(stderr,
> +				"The branch '%s' is not a strict subset of your current HEAD.\n"
> +				"If you are sure you want to delete it, run 'git branch -D %s'.\n",
> +				argv[i], argv[i]);
> +			exit(1);
> +		}
> +
> +		unlink(name);
> +		printf("Deleted branch %s.\n", argv[i]);
> +	}
> +}
> +
> +static int show_reference(const char *refname, const unsigned char *sha1)
> +{
> +	int is_head = !strcmp(refname, head);
> +
> +	printf("%c %s\n", (is_head ? '*' : ' '), refname);
> +
> +	return 0;

Unnecessary variable

> +}
> +
> +static void create_branch (const char *name, const char *start, int force)

No space

		David
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Received on Mon Aug 21 18:01:07 2006

This archive was generated by hypermail 2.1.8 : 2006-08-21 18:01:50 EST