Re: [PATCH] status: list unmerged files after staged files

Johannes Sixt [off-list ref] writes:

Moreover, keeping the unmerge entries at the top is inconvenient if a merge
touched many files, but only a few conflicted: After the conflicts were
resolved, the user will conduct a 'git add' command. In order to do that
with copy-and-paste, the user must scroll the terminal window up, and must
do so for each individual entry (because terminal windows commonly scroll
down automatically on the paste operation to make the cursor visible).

I actually was expecting that you would move this at the very bottom after
untracked list for the above reason, and also because this part is only
shown while running status (that was a good point you made in the previous
message) and never in commit.

The list of unmerged files is considered rather important because after
a conflicted merge they need attention. Since the output of git status does
not go through the pager, the end of the output remains immediately visible
in the terminal window. By placing unmerge entries at the end of the list,
the user can see them immediately.

Moreover, keeping the unmerge entries at the top is inconvenient if a merge
touched many files, but only a few conflicted: After the conflicts were
resolved, the user will conduct a 'git add' command. In order to do that
with copy-and-paste, the user must scroll the terminal window up, and must
do so for each individual entry (because terminal windows commonly scroll
down automatically on the paste operation to make the cursor visible).

Signed-off-by: Johannes Sixt <redacted>
---
On Dienstag, 1. September 2009, Junio C Hamano wrote:

Johannes Sixt [off-list ref] writes:

quoted

Moreover, keeping the unmerge entries at the top is inconvenient if a
merge touched many files, but only a few conflicted: After the conflicts
were resolved, the user will conduct a 'git add' command. In order to do
that with copy-and-paste, the user must scroll the terminal window up,
and must do so for each individual entry (because terminal windows
commonly scroll down automatically on the paste operation to make the
cursor visible).

I actually was expecting that you would move this at the very bottom after
untracked list for the above reason, and also because this part is only
shown while running status (that was a good point you made in the previous
message) and never in commit.

So you would not mind a more "drastic" change?

This version 2 can be regarded as a real improvement with the argument
above, whereas version 1 would only correct something of some
sort of regression, compared to v1.6.4.

(Originally I didn't dare to change too much and thought keeping staged
files together would make sense.)

-- Hannes

 wt-status.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 3395456..60d8425 100644
--- a/wt-status.c
+++ b/wt-status.c

@@ -561,7 +561,6 @@ void wt_status_print(struct wt_status *s)
 		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
 	}
 
-	wt_status_print_unmerged(s);
 	wt_status_print_updated(s);
 	wt_status_print_changed(s);
 	if (s->submodule_summary)

@@ -570,6 +569,7 @@ void wt_status_print(struct wt_status *s)
 		wt_status_print_untracked(s);
 	else if (s->commitable)
 		 fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked 

files)\n");
+	wt_status_print_unmerged(s);
 
 	if (s->verbose)
 		wt_status_print_verbose(s);
-- 
1.6.4.2.280.gb16ab

Johannes Sixt [off-list ref] writes:

The list of unmerged files is considered rather important because after
a conflicted merge they need attention. Since the output of git status does
not go through the pager, the end of the output remains immediately visible
in the terminal window. By placing unmerge entries at the end of the list,
the user can see them immediately.

Moreover, keeping the unmerge entries at the top is inconvenient if a merge
touched many files, but only a few conflicted: After the conflicts were
resolved, the user will conduct a 'git add' command. In order to do that
with copy-and-paste, the user must scroll the terminal window up, and must
do so for each individual entry (because terminal windows commonly scroll
down automatically on the paste operation to make the cursor visible).

Signed-off-by: Johannes Sixt <redacted>

On Dienstag, 1. September 2009, Junio C Hamano wrote:

quoted

I actually was expecting that you would move this at the very bottom after
untracked list for the above reason, and also because this part is only
shown while running status (that was a good point you made in the previous
message) and never in commit.

So you would not mind a more "drastic" change?

Well, it's not really about what _I_ like or mind.  It is primarily about
what the list collectively thinks.  I'd like to let other eyeballs and
brains to weigh in, as I am known to pick the worst layout from the UI
point of view as you saw in this thread already ;-).

(Originally I didn't dare to change too much and thought keeping staged
files together would make sense.)

Yes, unmerged ones are modified and the index knows about them, but you
haven't told git what you want to commit yet, so they are in the same
category as "changed but not updated" in that sense, but unlike "changed
but not updated", you cannot leave them as they are before proceeding, so
they are worse.

The "keeping related things together" argument does mean your v1 is better
than this patch, as you had "unmerged" next to "changed but not updated".
I personally think the "keep related things together" argument makes much
more sense than the "close to the bottom is easier to cut and paste"
argument, as I tend to focus at the top of the output when looking at the
status output and almost never cut & paste using mouse (screen for
rectangular cutting and pasting works wonderfully), but it probably is
just me.  And remember that I am only just one of the users, nothing more.

Sadly, "keep related things together" and "as close to the bottom as
possible" are not quite compatible, and we can pick one or the other, but
not both.

If I were to pick the middle ground, I would probably move it immediately
after the call to wt_status_print_changed(), with "keeping related things
together" as the primary justification.  It would be an incidental benefit
that it moves the part slightly closer to the bottom and gives it a better
chance of staying on the screen.

But I am not a great UI designer ;-)

On Tue, 01 Sep 2009, Junio C Hamano wrote:

Sadly, "keep related things together" and "as close to the bottom as
possible" are not quite compatible, and we can pick one or the other, but
not both.

If I were to pick the middle ground, I would probably move it immediately
after the call to wt_status_print_changed(), with "keeping related things
together" as the primary justification.  It would be an incidental benefit
that it moves the part slightly closer to the bottom and gives it a better
chance of staying on the screen.

I can only speak of my personal experience that during rebase -i,
there is no (or very few) untracked files in the list so that the
sequence  "modified, unmerged, untracked" is also a good alternative.

(I hope the mail-followup-to is correct this time)

-- 
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3

On Tue, Sep 01, 2009 at 05:18:40PM -0700, Junio C Hamano wrote:

The "keeping related things together" argument does mean your v1 is better
than this patch, as you had "unmerged" next to "changed but not updated".
I personally think the "keep related things together" argument makes much
more sense than the "close to the bottom is easier to cut and paste"
argument, as I tend to focus at the top of the output when looking at the
status output and almost never cut & paste using mouse (screen for
rectangular cutting and pasting works wonderfully), but it probably is
just me.  And remember that I am only just one of the users, nothing more.

Sadly, "keep related things together" and "as close to the bottom as
possible" are not quite compatible, and we can pick one or the other, but
not both.

Just my two cents (and I think I have as good a track record at UI
design as Junio... ;) ):

I think "related things together" trumps "close to the bottom". Because
the former is something that _always_ applies to your output, while the
latter is catering to a particular use case and a particular screen
setup.

In other words, why is the _bottom_ reserved for more important things
instead of the _top_? If I have a tall terminal that is long enough to
see the output, are you potentially making the important thing less
obvious (because I tend to read the the output from top to bottom)? If I
use a pager (either manually, because I have seen that the output is too
long, or automatically via the pager.status config variable)? What about
reading status output into an interface wrapper like "tig status"?

So while you may be helping some users, I tend to think you may be
hurting others.

-Peff

PS I am also not entirely convinced that unmerged entries are somehow
more important to call attention to in the list than other entries. But
the above argues that even _if_ you think they are more important, it is
still not necessarily a good thing to move them to the bottom.

Jeff King [off-list ref] writes:

I think "related things together" trumps "close to the bottom". Because
the former is something that _always_ applies to your output, while the
latter is catering to a particular use case and a particular screen
setup.

In other words, why is the _bottom_ reserved for more important things
instead of the _top_? If I have a tall terminal that is long enough to
see the output, are you potentially making the important thing less
obvious (because I tend to read the the output from top to bottom)? If I
use a pager (either manually, because I have seen that the output is too
long, or automatically via the pager.status config variable)? What about
reading status output into an interface wrapper like "tig status"?

Yes and no.

Sure, I always work in a 92x70 screen session with 10k lines of scrollback
buffer, and when I cut and paste I do not use a mouse but use screen's cut
buffer, so I would have no problem with the list at the top.

Not that I would use "git status" while resolving merges---I would use
"ls-files -u" myself, and I may perhaps start using "status -suno", so my
personal preference does not really count on this topic.

But not everybody is used to such a set-up.  If you rely on terminal's
scrollback buffer with mouse and a short terminal, I can see cutting and
pasting would be an issue.  I do not have a good answer to "tig status",
but the design principle of supporting the lowest denominator is
important.

J6t made a good point that this new section won't appear when committing,
which I didn't take account when I was first explained how the ordering
was chosen.  After thinking about this a bit more, I think "untracked" and
"modified but not updated" sections, unlike when recording your own
commit, is mostly uninteresting while resolving a merge.  You never add
files that you forgot to add to a merge; nor you would add your local
modifications to a merge.  So the only sections that are interesting are
this new "unmerged" section and "updated" section to see the extent of
damage the merge causes to your history by introducing the crap other
people dumped on you ;-) [*1*].

The above suggests me that (1) we would want to have the new "unmerged"
section next to "updated" section, (2) we would want to have it later in
the output rather than earlier, and (3) in the traditional output, people
are used to see unmerged paths in "changed" section, so it would be easier
for them to transition if "unmerged" section were near "changed" section.

That makes the ideal place between updated and changed, no?

Incidentally that is where J6t's first patch was.  So I would agree with
the patch (but not necessarily with its justification).

[1] It might even make sense to omit other sections and show only
"updated" and "unmerged" in this order when the index is unmerged, but
that is a lot more drastic change for 1.7.0.

On Tue, Sep 01, 2009 at 09:26:26PM -0700, Junio C Hamano wrote:

But not everybody is used to such a set-up.  If you rely on terminal's
scrollback buffer with mouse and a short terminal, I can see cutting and
pasting would be an issue.  I do not have a good answer to "tig status",
but the design principle of supporting the lowest denominator is
important.

But I'm not sure it is about "lowest common denominator". I think it is
about different people having different preferences (as a matter of
fact, I use an 80x25 terminal most of the time, and I think I prefer the
content at the top. Perhaps it is simply habit, but I do think having it
right next to "staged for commit" items makes the most sense).

The above suggests me that (1) we would want to have the new "unmerged"
section next to "updated" section, (2) we would want to have it later in
the output rather than earlier, and (3) in the traditional output, people
are used to see unmerged paths in "changed" section, so it would be easier
for them to transition if "unmerged" section were near "changed" section.

That makes the ideal place between updated and changed, no?

Yes, I think that is fine, and makes more sense than where we have it
now. I mainly wanted to argue against sticking it at the very bottom.

[1] It might even make sense to omit other sections and show only
"updated" and "unmerged" in this order when the index is unmerged, but
that is a lot more drastic change for 1.7.0.

I think that is a really bad idea. The mental model of "git status"
(versus individual diff or ls-files commands) is to see _everything_
going on in the repo. Showing a subset breaks that model and gives a
false sense of what is actually happening.

I don't know that it would matter much most of the time anyway. If you
have unmerged entries, you probably don't have any (or many) "changed
but not updated" files, too (since you are not working on a new commit
but rather a merge, they would have to be dirty state you are carrying
permanently, but not related to the merge). If you do, you probably want
to see them to be aware of what is going on.

You probably also don't have a lot of untracked files. If you have a
few, you might want to be reminded of them to make sure they were not
something you were preparing to help with a tricky merge. And if you are
the sort of person who carries around a lot of untracked files, and for
some reason you refuse to put them in your .gitignore, then you probably
have status.untracked set to "no" already (or you should consider
setting it), as they will be bugging you in other situations, as well.

-Peff

On Wed, Sep 02, 2009 at 01:12:48AM -0400, Jeff King wrote:

On Tue, Sep 01, 2009 at 09:26:26PM -0700, Junio C Hamano wrote:

quoted

[1] It might even make sense to omit other sections and show only
"updated" and "unmerged" in this order when the index is unmerged, but
that is a lot more drastic change for 1.7.0.

I think that is a really bad idea. The mental model of "git status"
(versus individual diff or ls-files commands) is to see _everything_
going on in the repo. Showing a subset breaks that model and gives a
false sense of what is actually happening.

It would be nice to be able to explicitly ask to suppress some of the
output for cases where there's a lot of it and only a small part is
interesting (like when resolving a large merge as mentioned earlier) - I
often end up doing this by hand in those situations.  I do agree that
doing this by default would be surprising.

On Wed, Sep 02, 2009 at 01:48:32PM +0100, Mark Brown wrote:

It would be nice to be able to explicitly ask to suppress some of the
output for cases where there's a lot of it and only a small part is
interesting (like when resolving a large merge as mentioned earlier) - I
often end up doing this by hand in those situations.  I do agree that
doing this by default would be surprising.

Yeah, we already have --untracked-files=<no|normal|all> and a matching
config variable. If there are cases people find useful, I don't see a
reason why we can't make other sections configurable, too. I think it
just somebody to write a patch for the behavior they think makes sense
(or at the very least a concrete proposal).

-Peff

On Wed, Sep 02, 2009 at 02:00:50PM -0400, Jeff King wrote:

Yeah, we already have --untracked-files=<no|normal|all> and a matching
config variable. If there are cases people find useful, I don't see a
reason why we can't make other sections configurable, too. I think it
just somebody to write a patch for the behavior they think makes sense
(or at the very least a concrete proposal).

My main wishlist would be to have the same control for the changes to be
committed for the big merge case, the use case being while resolving
merges where those changes are those that have been dealt with and the
remaining (hopefully much fewer) changes are those that still need
attention.

On Wed, Sep 02, 2009 at 07:39:23PM +0100, Mark Brown wrote:

My main wishlist would be to have the same control for the changes to be
committed for the big merge case, the use case being while resolving
merges where those changes are those that have been dealt with and the
remaining (hopefully much fewer) changes are those that still need
attention.

I think we need to be more concrete than that. What is the "big merge
case"? If there are any unmerged paths?

What exactly should be cut out, and how can it be configured? Should you
have "status.unmerged" to cut out certain things? Which things (of
staged, unstaged, and untracked)? Or should it go the other way, with a
status.showStaged variable which can be set to "always", "never", or
"unmerged" (and probably adding an "unmerged" option to
"status.showUntrackedFiles).

-Peff

On Sat, Sep 05, 2009 at 05:04:22AM -0400, Jeff King wrote:

On Wed, Sep 02, 2009 at 07:39:23PM +0100, Mark Brown wrote:

quoted

My main wishlist would be to have the same control for the changes to be
committed for the big merge case, the use case being while resolving

I think we need to be more concrete than that. What is the "big merge
case"? If there are any unmerged paths?

The context was that this was done when explictly requested by the user
so all the time when enabled.  In the context I'm thinking of this would
be used via the command line more than via the config file.

What exactly should be cut out, and how can it be configured? Should you
have "status.unmerged" to cut out certain things? Which things (of
staged, unstaged, and untracked)? Or should it go the other way, with a
status.showStaged variable which can be set to "always", "never", or
"unmerged" (and probably adding an "unmerged" option to
"status.showUntrackedFiles).

I'd been thinking of not showing anything in the index but keeping
everything else.  In terms of a configuration variable I'd go with
specifying the things not to show rather than the things to show - 
the noise to cut out.