All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael J Gruber <git@drmicha.warpmail.net>
Cc: git@vger.kernel.org, Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: Re: [PATCHv3 0/3]More diffs for commit/status
Date: Thu, 05 Mar 2015 12:15:16 -0800	[thread overview]
Message-ID: <xmqqlhjb2obv.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqtwxz2qmb.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Thu, 05 Mar 2015 11:25:48 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> - Do we want the header line also for "status -v"? (I would say yes, but that
>>   would be a change to current behaviour.)
>
> I would not object to it very strongly, but I do not see a point in
> changing the behaviour.
>
> And I do not see why a new user would want it anyway.  There is no
> need to differenciate the changes to be committed from the changes
> left in the working tree when the latter is not even shown.

Extending this line of thought further.

If I am reading your patch 3/3 right, "status -v -v" shows the
header when there are patches to be shown for the category.  I am
not sure if that is the most helpful way for the users, when either
c/i xor i/w diffs is missing.

There are four cases, obviously ;-)

1. When there are changes to be committed:

 a) When there is no change left in the working tree, the proposed
    output would be the same as the more familiar "status -v"
    output.  Showing changes to be committed header would of course
    help.

    I wondered if the proposed behaviour hurts the user by hiding
    the header for changes to be left out, though.  By seeing that
    the second header alone and no diff, the user will be assured
    that there is no changes left in the working tree, forgotten to
    be added.  But this point is minor.  As the users get used to
    the behaviour of "-v -v", they will learn to read the emptyness
    and find its proper meaning that there is no change left out.
    So I think the proposed behaviour would be OK in this case.  In
    fact, not showing the second header when there is no change left
    in the working tree will help potential issues with case 2-b).

 b) When there is change left in the working tree, the proposed
    output is fine.  Two headers are shown to indicate what the
    following diff is about and cleanly shows where the boundary of
    the two classes are (especially if you resurrect the -{50}
    separator line I suggested, at least for the second header).


2. When there is no change to be committed:

 a) When there is no change left in the working tree, the proposed
    output is fine.  There is no output (no header, no diff), and
    the user immediately knows that the working tree and the index
    are clean.

 b) When there are changes left in the working tree, the user sees
    one header followed by a diff in the proposed output.  Visually,
    the single line heading (even with the separateor line) may be
    so small in the context of the whole output, and the user needs
    to READ it to notice that the diff being shown are not what is
    going to be committed.  In other words, it is too similar to the
    proposed output in case 1-a).

    If we show the "to be committed" header followed by no diff, and
    then the second header followed by diff, it would be crystial
    clear to the user, because it looks unusual, that what is shown
    is different from case 1-a).  This would especially be true if
    you resurrected -{50} separator line after the heading.


So, my recommendation for "status -v -v" would be:

    if (there are changes to be committed, or
	there are changes left in the working tree) {
	show "to be committed" with -{50};
        show c/i diff;
    }
    if (there are changes left in the working tree) {
	show "left in the working tree" with -{50};
        show i/w diff;
    }

  reply	other threads:[~2015-03-05 20:15 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-13  8:56 How to prevent empty git commit --amend Ivo Anjo
2015-01-13  8:59 ` Daniel Knittl-Frank
2015-01-13 10:22   ` Ivo Anjo
2015-01-13 11:20     ` Michael J Gruber
2015-01-14 10:00 ` Matthieu Moy
2015-01-14 12:15   ` Ivo Anjo
2015-01-14 12:45     ` Matthieu Moy
2015-01-14 17:27   ` Junio C Hamano
2015-01-14 17:36     ` Junio C Hamano
2015-01-15 16:08       ` [RFC/PATCH] commit/status: show the index-worktree with -v -v Michael J Gruber
2015-01-15 20:11         ` Junio C Hamano
2015-01-15 20:38           ` Junio C Hamano
2015-01-16  8:13           ` Michael J Gruber
2015-03-03 14:16             ` [PATCHv2 0/2] More diffs for commit/status Michael J Gruber
2015-03-03 14:16               ` [PATCHv2 1/2] t7508: test git status -v Michael J Gruber
2015-03-03 21:20                 ` Junio C Hamano
2015-03-03 22:26                   ` Junio C Hamano
2015-03-04 11:05                     ` Michael J Gruber
2015-03-04 21:27                       ` Junio C Hamano
2015-03-03 14:16               ` [PATCHv2 2/2] commit/status: show the index-worktree diff with -v -v Michael J Gruber
2015-03-03 21:26                 ` Junio C Hamano
2015-03-04 11:11                   ` Michael J Gruber
2015-03-04 21:13                     ` Junio C Hamano
2015-03-05 14:13                       ` [PATCHv3 0/3]More diffs for commit/status Michael J Gruber
2015-03-05 14:13                         ` [PATCHv3 1/3] t7508: .gitignore 'expect' and 'output' files Michael J Gruber
2015-03-05 14:13                         ` [PATCHv3 2/3] t7508: test git status -v Michael J Gruber
2015-03-05 14:13                         ` [PATCHv3 3/3] commit/status: show the index-worktree diff with -v -v Michael J Gruber
2015-03-05 19:25                         ` [PATCHv3 0/3]More diffs for commit/status Junio C Hamano
2015-03-05 20:15                           ` Junio C Hamano [this message]
2015-03-05 20:27                             ` 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=xmqqlhjb2obv.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@imag.fr \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    /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.