Re: [PATCHv3 0/3]More diffs for commit/status

2015-03-05 Thread Junio C Hamano
Michael J Gruber g...@drmicha.warpmail.net writes:

 v3 has the following changes:
 - new leading patch by Junio to clean up t7508 (slightly modified by myself)
 - adjust tests accordingly
 - revert back to standard c/,i/ resp. i/,w/ diff prefixes with a header line

 Open questionis for 3/3:
 - Do we need the header to stick out even more? (I don't think so, although
   having the STATUS_HEADER color to be different may help.)

If we have more than one paths in each category, I would think at
least a separator line (I used -{50} in my illustration you are
replying to) before the verbal Changes to be committed would help.

 - 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.
--
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


Re: [PATCHv3 0/3]More diffs for commit/status

2015-03-05 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 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.
 ...
 So, my recommendation for status -v -v would be:

Taking the conclusion part of what I said back.  I think the exact
same reasoning will lead to a much simpler and more concise output
by (1) using exactly the same logic you have in 3/3 to decide when
to show or not show the headers and (2) adding the ^-{50}$ separator
only before the second header that is shown before the changes left
in the working tree.

Then, 1-a) will show the same output as status -v, 1-b) will start
as the same as status -v, followed by a visually significant
separator followed by diff, 2-a) will be empty, and 2-b) will start
with a visually significant and unusual separator line before the
diff.  That would make 1-a) and 2-b) visually very distinct and
reduce the chance of confusion.

The updated outline for status -v -v would be:

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

Thanks.
--
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


Re: [PATCHv3 0/3]More diffs for commit/status

2015-03-05 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Michael J Gruber g...@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;
}

--
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