liam Beguin [off-list ref] 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".
quoted hunk
+}
+
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.
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 [off-list ref] writes:
quoted
+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.
quoted
+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.
quoted
+ 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".
quoted
+}
+
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
On Mon, Jun 12, 2017 at 11:42:44PM -0400, liam Beguin wrote:
[...]
quoted
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?
The same that exists between an interface and a concrete implementation
in a programming language.
"A stash entry" is a concept which is defined to keep explicitly
recorded untracked files and which can be applied, shown and deleted
from the stash bag (well, you can create a branch off it as well).
The fact a stash entry is a merge commit of two synthetic commits is an
implementation detail. It can be very useful at times for power users,
but regular Git users need not be concerned with this.
Another fact worth reiterating that what the UI displays to the user is
better to match what the user reads in the docs. ;-)
Hi,
On 13/06/17 02:42 AM, Konstantin Khomoutov wrote:
On Mon, Jun 12, 2017 at 11:42:44PM -0400, liam Beguin wrote:
[...]
quoted
quoted
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?
The same that exists between an interface and a concrete implementation
in a programming language.
Makes sense, I thought there was a more fundamental difference.
"A stash entry" is a concept which is defined to keep explicitly
recorded untracked files and which can be applied, shown and deleted
from the stash bag (well, you can create a branch off it as well).
I've noticed this but I don't understand when it can be used.
I'll try to find out more on this.
The fact a stash entry is a merge commit of two synthetic commits is an
implementation detail. It can be very useful at times for power users,
but regular Git users need not be concerned with this.
Another fact worth reiterating that what the UI displays to the user is
better to match what the user reads in the docs. ;-)
I'll make changes as suggested by Junio. I slightly prefer
"Your stash has %d entry/entries" over "You have %d stash/stashes"
but I'll go with what's used elsewhere in the documentation.
Thanks,
- Liam