From: Junio C Hamano <gitster@pobox.com>
To: liam Beguin <liambeguin@gmail.com>
Cc: Jeff King <peff@peff.net>, Samuel Lijin <sxlijin@gmail.com>,
Houston Fortney <houstonfortney@gmail.com>,
"git\@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: Feature Request: Show status of the stash in git status command
Date: Mon, 12 Jun 2017 08:35:24 -0700 [thread overview]
Message-ID: <xmqqpoe9p6bn.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1c04d689-6796-17d1-e058-18874768c22a@gmail.com> (liam Beguin's message of "Sun, 11 Jun 2017 13:07:03 -0400")
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
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.
next prev parent reply other threads:[~2017-06-12 16:45 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-07 22:46 Feature Request: Show status of the stash in git status command Houston Fortney
2017-06-10 8:25 ` Jeff King
2017-06-10 10:12 ` Samuel Lijin
2017-06-10 10:22 ` Jeff King
2017-06-11 17:07 ` liam Beguin
2017-06-11 17:57 ` Randall S. Becker
2017-06-11 18:18 ` Igor Djordjevic
2017-06-11 18:30 ` Randall S. Becker
2017-06-12 15:35 ` Junio C Hamano [this message]
2017-06-13 3:42 ` liam Beguin
2017-06-13 6:42 ` Konstantin Khomoutov
2017-06-13 12:34 ` liam Beguin
2017-06-13 13:06 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqpoe9p6bn.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=houstonfortney@gmail.com \
--cc=liambeguin@gmail.com \
--cc=peff@peff.net \
--cc=sxlijin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.