Phil Hord [off-list ref] writes:
+ merge a git-merge is in progress
+ am a git-am is in progress
+ rebase a git-rebase is in progress
+ rebase-interactive a git-rebase--interactive is in progress
+ cherry-pick a git-cherry-pick is in progress
+ bisect a git-bisect is in progress
Avoid using git-foo syntax in documentation, it suggests that this is
valid command, which isn't true anymore. `git foo` seems the most common
syntax. Also, git-rebase--interactive is not user-visible => `git rebase
--interactive`.
- if (state->am_empty_patch)
+ if (state->substate==WT_SUBSTATE_NOMINAL)
status_printf_ln(s, color,
_("The current patch is empty."));
This looks weird. First, spaces around == (here and below). Then, the
logic is unintuitive. The "if" suggests everything is allright, and the
message below is very specific. This at least deserves a comment.
if (advice_status_hints) {
- if (!state->am_empty_patch)
+ if (state->substate==WT_SUBSTATE_CONFLICTED)
Spaces around ==.
+static void wt_print_token(struct wt_status *s, const char *color, const char *token)
+{
+ color_fprintf(s->fp, color, "%s", token);
+ fputc(s->null_termination ? '\0' : '\n', s->fp);
+}
The output format seems to be meant only for machine-consumption. Is
there any case when we'd want color? I'd say we can disable coloring
completely for this format (normally, color=auto does the right thing,
but I prefer being 100% sure I'll get no color when writing scripts)
+static void wt_shortstatus_print_sequencer(struct wt_status *s)
[...]
+void wt_sequencer_print(struct wt_status *s)
+{
+ wt_shortstatus_print_sequencer(s);
+}
+
Why do you need this trivial wrapper?
Other than that, I like the idea (although I have no concrete use-case
in mind), but I didn't actually test the patch.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Matthieu Moy wrote:
Phil Hord [off-list ref] writes:
quoted
+ merge a git-merge is in progress
+ am a git-am is in progress
+ rebase a git-rebase is in progress
+ rebase-interactive a git-rebase--interactive is in progress
+ cherry-pick a git-cherry-pick is in progress
+ bisect a git-bisect is in progress
Avoid using git-foo syntax in documentation, it suggests that this is
valid command, which isn't true anymore. `git foo` seems the most common
syntax. Also, git-rebase--interactive is not user-visible => `git rebase
--interactive`.
Thanks.
quoted
- if (state->am_empty_patch)
+ if (state->substate==WT_SUBSTATE_NOMINAL)
status_printf_ln(s, color,
_("The current patch is empty."));
This looks weird. First, spaces around == (here and below). Then, the
logic is unintuitive. The "if" suggests everything is allright, and the
message below is very specific. This at least deserves a comment.
Yes, I agree. It was less clear but more reasonable before I tried to
clear it up some. It's driven by the short-token printer. The state is
"you're in a 'git am' but I do not see any conflicted files. Therefore,
your patch must be empty." I suspect this may be a leaving from the
git-am machinery where it did not choose to clean up after itself if the
patch was empty, but perhaps it should have. I seldom use git-am, so I
do not know if this code reflects a common and useful state.
I'll try to make this more explicit. Currently the short-status
version will say either "am" or "am \n conflicted" when a 'git am' is in
progress. The logical path to follow if I re-add 'git-am-empty' state
tracker is for this to now show either "am \n am-is-empty" or "am \n
conflicted". But I think I should suppress the "am-is-empty" report in
that case. What do you think
quoted
if (advice_status_hints) {
- if (!state->am_empty_patch)
+ if (state->substate==WT_SUBSTATE_CONFLICTED)
Spaces around ==.
quoted
+static void wt_print_token(struct wt_status *s, const char *color, const char *token)
+{
+ color_fprintf(s->fp, color, "%s", token);
+ fputc(s->null_termination ? '\0' : '\n', s->fp);
+}
The output format seems to be meant only for machine-consumption. Is
there any case when we'd want color? I'd say we can disable coloring
completely for this format (normally, color=auto does the right thing,
but I prefer being 100% sure I'll get no color when writing scripts)
Originally I had this output nested in the normal 'git status --short'
output, like a shortened form of the "advice". Then, 'git-status
--porcelain' would show the tokens without color, but 'git status
--short' would show them with color. I thought I might be going back
there, or that I might combine this with full 'git status' again
somehow, and colors seemed appropriate still.
The --short status report is too confusing when tokens may or may-not
appear, and it would likely break some scripts, even though they should
be using --porcelain. And the full status already has its long versions
of the same text. So I can remove this color decorator until someone
finds a need for it.
quoted
+static void wt_shortstatus_print_sequencer(struct wt_status *s)
[...]
quoted
+void wt_sequencer_print(struct wt_status *s)
+{
+ wt_shortstatus_print_sequencer(s);
+}
+
Why do you need this trivial wrapper?
Another left-over from its previous multiple versions. I'll simplify it.
Other than that, I like the idea (although I have no concrete use-case
in mind), but I didn't actually test the patch.
My own use-case involves $PS1. I keep running into "you cannot
cherry-pick because you are in the middle of a rebase" in submodules in
which I have long forgotten about the failed action and have gone on to
write many new commits, or switched branches, or worse. I do not know
what 'git rebase --abort' will give me in those cases, and I wonder what
work I might have lost for having been interrupted in the middle of that
action in the past. These tokens will help me decorate my prompt to
remind me I left some baggage untended.
Thanks for the feedback.
Phil