Phil Hord <ho...@cisco.com> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to