[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH v2] status: display "doing what" information in git
From:       Junio C Hamano <gitster () pobox ! com>
Date:       2011-05-06 18:24:03
Message-ID: 7vaaez1yd8.fsf () alter ! siamese ! dyndns ! org
[Download RAW message or body]

Pierre Habouzit <madcoder@debian.org> writes:

>> > +	const char * const rebase_advice =
>> > +		_("use \"git rebase --abort\" to abort current rebase or proceed");
>> > +	const char * const am_advice =
>> > +		_("use \"git am --abort\" to abort current mailbox apply or proceed");
>> 
>> If the reader does not even know that the "--abort" option is used to
>> abort, " or proceed" needs to be followed by "by doing $this", if it wants
>> to have any practical value.  I would suggest dropping it.
>> 
>> > +	const char * const merge_advice =
>> > +		_("use \"git reset --hard\" to abort, or resolve conflicts and commit");
>> This codepath being in wt-status.c, I wonder if any of these advices could
>> trigger to appear in the log message editor when the user tries to run
>> "git commit".  If so, I suspect any of the above is too late to help the
>> user, no?
>
> Well I'm not very happy with the advices, feel free to reword them in a better
> way.

I already suggested dropping it ;-)  Rewording to emptiness would be the
simplest and cleanest.

>> Also, should we make these conditional upon advice.status or something?
> they are
>                           vvvvvvvvvvvvvvvvvvv
>     +       if (advice && advice_status_hints) {
>     +               status_printf(s, color(WT_STATUS_HEADER, s), "");
>     +               status_printf_more(s, status_nobranch, "  (%s)\n", advice);
>     +       }

By the way, this change also breaks t7060, at least.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic