Re: [PATCH 2/8] update-index: use enum for untracked cache options

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 8, 2015, 19:11 UTC
Message-ID: <xmqq1tawlpqw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1449594916-21167-3-git-send-email-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

Show 21 quoted lines

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  builtin/update-index.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 6f6b289..246b3d3 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
>  #define UNMARK_FLAG 2
>  static struct strbuf mtime_dir = STRBUF_INIT;
>  
> +/* Untracked cache mode */
> +enum uc_mode {
> +	UNDEF_UC = -1,
> +	NO_UC = 0,
> +	UC,
> +	FORCE_UC
> +};
> +

With these, the code is much easier to read than with the mystery constants, but did you consider making UC_ a common prefix for further ease-of-reading? E.g.

    UC_UNSPECIFIED
    UC_DONTUSE
    UC_USE
    UC_FORCE

or something?

Show 43 quoted lines

>  __attribute__((format (printf, 1, 2)))
>  static void report(const char *fmt, ...)
>  {
> @@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
>  int cmd_update_index(int argc, const char **argv, const char *prefix)
>  {
>  	int newfd, entries, has_errors = 0, line_termination = '\n';
> -	int untracked_cache = -1;
> +	enum uc_mode untracked_cache = UNDEF_UC;
>  	int read_from_stdin = 0;
>  	int prefix_length = prefix ? strlen(prefix) : 0;
>  	int preferred_index_format = 0;
> @@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		OPT_BOOL(0, "untracked-cache", &untracked_cache,
>  			N_("enable/disable untracked cache")),
>  		OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
> -			    N_("enable untracked cache without testing the filesystem"), 2),
> +			    N_("enable untracked cache without testing the filesystem"), FORCE_UC),
>  		OPT_END()
>  	};
>  
> @@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		the_index.split_index = NULL;
>  		the_index.cache_changed |= SOMETHING_CHANGED;
>  	}
> -	if (untracked_cache > 0) {
> +	if (untracked_cache > NO_UC) {
>  		struct untracked_cache *uc;
>  
> -		if (untracked_cache < 2) {
> +		if (untracked_cache < FORCE_UC) {
>  			setup_work_tree();
>  			if (!test_if_untracked_cache_is_supported())
>  				return 1;
> @@ -1123,7 +1131,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		add_untracked_ident(the_index.untracked);
>  		the_index.cache_changed |= UNTRACKED_CHANGED;
>  		fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
> -	} else if (!untracked_cache && the_index.untracked) {
> +	} else if (untracked_cache == NO_UC && the_index.untracked) {
>  		the_index.untracked = NULL;
>  		the_index.cache_changed |= UNTRACKED_CHANGED;
>  		fprintf(stderr, _("Untracked cache disabled\n"));