Re: [PATCH 5/6] builtin-commit.c: further refactor branch switching

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Dec 6, 2008, 02:08 UTC
Message-ID: <alpine.LFD.2.00.0812051802570.3425@nehalem.linux-foundation.org>
In-Reply-To: <1228528455-3089-6-git-send-email-gitster@pobox.com>

The whole series looks good, I think.

But one thing I reacted to is exemplified best by this one:

On Fri, 5 Dec 2008, Junio C Hamano wrote:

>
> +int reset_index_and_worktree(int force, int merge, int quiet, int *wt_error,
> +			     struct commit *old_commit, const char *old_label,
> +			     struct commit *new_commit, const char *new_label)

This takes three flags (force, merge, quiet) as three parameters.

And in the series, 3/6 and 4/6 had other cases where the same flags (just not _all_ of them) were passed around to other functions. In 4/6, you had "switch_trees()" taking "int merge, int quiet", and in 3/6 you had "reset_tree()" taking "int quiet" and also have a "int worktree" flag.

I think it would be really nice to have a unified namespace for these flags. There are lots of commonalities between "checkout" and "reset" (and certainly differences too), wouldn't it be nice to have something like

	#define CO_FORCE 0x001
	#define CO_MERGE 0x002
	#define CO_QUIET 0x004
	#define CO_WORKTREE 0x008

and just pass in a single "unsigned int checkout_flags" as an argument?

Having eight parameters is obscene.

		Linus