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

[liam Beguin <liambeguin@gmail.com>]

Hi, 

Thanks for the feedback. I'll be sending a patch with the updates shortly!

On 12/06/17 11:35 AM, Junio C Hamano wrote:
> liam Beguin <liambeguin@gmail.com> writes:
> 
>> +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.
> 
>> +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.
> 
>> +	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

what's different between a stash and a commit? 

> 
> 	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".
> 
>> +}
>> +
>>  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.
> 

Thanks,

 - Liam