Re: Feature Request: Show status of the stash in git status command

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 12, 2017, 15:35 UTC
Message-ID: <xmqqpoe9p6bn.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1c04d689-6796-17d1-e058-18874768c22a@gmail.com>

liam Beguin <liambeguin@gmail.com> writes:

Show 8 quoted lines

> +static int stash_count_refs(struct object_id *ooid, struct object_id *noid,
> +			    const char *email, timestamp_t timestamp, int tz,
> +			    const char *message, void *cb_data)
> +{
> +	int *c = cb_data;
> +	(*c)++;
> +	return 0;
> +}

Count up, and tell the caller to keep going by returning 0. That sounds sane.

Show 5 quoted lines

> +static void wt_longstatus_print_stash_summary(struct wt_status *s)
> +{
> +	int stash_count = 0;
> +
> +	for_each_reflog_ent("refs/stash", stash_count_refs, &stash_count);

And do so with a counter initialized to 0.  Also sane.

Show 5 quoted lines

> +	if (stash_count > 0)
> +		status_printf_ln(s, GIT_COLOR_NORMAL,
> +				 Q_("Your stash currently has %d commit",
> +				    "Your stash currently has %d commits", stash_count),
> +				 stash_count);

Conceptually, the contents of the stash are *not* commits, even though the implementation happens to use a commit to represent each stash entry. Perhaps "has %d entry/entries" is an improvement, but a quick scanning of an early part of "git stash --help" tells me that

	You have 1 stash / You have 4 stashes

would be the best, as the documentation calls each entry "a stash". E.g. "list" is explained to list "the stashes", and "show <stash>" is explained to show the changes recorded in "the stash".

Show 20 quoted lines

> +}
> +
>  static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncommitted)
>  {
>  	struct child_process sm_summary = CHILD_PROCESS_INIT;
> @@ -1536,6 +1557,7 @@ static void wt_longstatus_print(struct wt_status *s)
>  	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
>  	const char *branch_status_color = color(WT_STATUS_HEADER, s);
>  	struct wt_status_state state;
> +	int show_stash = 0;
>  
>  	memset(&state, 0, sizeof(state));
>  	wt_status_get_state(&state,
> @@ -1641,6 +1663,8 @@ static void wt_longstatus_print(struct wt_status *s)
>  		} else
>  			printf(_("nothing to commit, working tree clean\n"));
>  	}
> +	if (!git_config_get_bool("status.showStash", &show_stash) && show_stash)
> +		wt_longstatus_print_stash_summary(s);
>  }

Try to get "status.showstash" as a boolean, and only when it succeeds and the value is true, give this extra info (i.e. when the variable does not exist, do not complain and do not show). Sounds sensible.

Overall the logic looks good to me; just the phrasing is questionable, relative to the existing documentation.

Thanks.