Re: Two ideas for improving git's user interface

[Junio C Hamano <junkio@cox.net>]

Carl Worth <cworth@cworth.org> writes:

> If not, we should be able to simplify things since a lot of the
> UI complexity being discussed (-a vs. no -a, path names vs. no path
> names), hinges on the handling of skewed files.

I am in agreement with you that "skewed files" might lead to
confusion, but I do not see how that relates to "-a vs no -a" nor
"path names vs no path names" issues.

Let's say we try to detect and forbid committing skewed files.  How
would we do that?  For the sake of clarity, let's say we fixed the
commit command the way I said in the message you are responding.

Now:

1. "git commit" is the traditional one; it commits the current index.
   We enumerate paths that 'git-diff-index --cached --name-only HEAD'
   tells are different (they are the paths to be committed -- what
   about merges?  Maybe take union from all parents?).  Then we see if
   the paths from "git-diff-files --name-only" (locally modified
   files) overlap with them.  Overlapping ones will be skewed if we
   make a commit.

2. "git commit --also fileA..." updates fileA... on top of the current
   index and commits that.  After doing "git update-index fileA...",
   the story is the same as the previous case.

3. "git commit fileA..." initializes a temporary index from the
   current HEAD, updates fileA... and commits that.  We would need a
   check to make sure index matches HEAD at specified paths, but after
   that check passes, there is no skewed files being committed and
   there is nothing more to check.

4. "git commit -a" by definition would not have skewed files and there
   is nothing to check.

So what you say sounds doable.  But I wonder if that really helps
much.

Let's say we want to give an interface to a class of users who do
_not_ want to worry about the presense of the index file.  That means
they will _never_ run "git update-index" themselves, although "git
commit", "git add", and "git merge" may run update-index for them
internally.  Essentially, you tell them to always use "git commit -a"
or "git commit fileA...", and do not teach them "git commit", "git
commit --also fileA...".  IOW they will be doing only 3 or 4.  In this
case, we do not need any of the "skewed files" check.

The extra checks in 1 and 2 would prevent index-unaware users from
making obvious mistakes, but if they do not understand index then they
would still be surprised anyway.  For example, "git commit" commits
the files they previously run "git add" on, but leaves other modified
files in the working tree uncommitted.  This is different from either
3 or 4 that they have learned so far.  If they did "git commit fileA",
the file earlier they run "git add" is not committed.  If they did
"git commit -a", files other than the added files are also committed.
So in that sense the above checks are doable but I do not think it
helps that much to alleviate the confusion.

These extra checks in 1 and 2 may protect index-aware users from
making mistakes, to a certain degree.  I am not convinced enough
myself to pay the cost of extra checks, though, because my workflow is
to do the final review exactly like what you said below.

> My workflow has been to always perform a final review of such a diff
> while composing the commit message. I'd like to be able to do that
> with git.

That matches my workflow.  I do either one of these (I never use "git
commit paths..."):

	$ work work work
        $ I may do update-index [--add|--remove] here
        $ git diff --cached
        $ git commit

	$ work work work
        $ I may do update-index [--add|--remove] here
        $ git diff HEAD
        $ git commit -a

In either cases "skewed files" do not matter.  This can be summarized
in a short paragraph:

	If you are going to commit with "git commit" (no parameters),
	check the final result with "git diff --cached".  If you are
	going to commit with "git commit -a", check with "git diff
	HEAD".

I said why I do not do "git commit paths..." myself, but I think this
"skewed files" discussion adds another thing to be careful about if
you use it.  If you do this (with the current tool, you drop --also):

	$ work on file A
        $ git diff A
        ... that looks fine so far ...
        $ git update-index A
        $ work more on file A
        $ git diff A
        ... incrementally that looks fine ...
        $ git commit --also A

you would end up commiting something you have not done the "final
review".  You need to have the final check before such a commit:

	$ work on file A
        $ git diff A
        ... that looks fine so far ...
        $ git update-index A
        $ work more on file A
        $ git diff A
        ... incrementally that looks fine ...
 +++++  $ git diff HEAD
        $ git commit --also A

This includes all changes that are not in the index and are not going
to be included in the commit (i.e. changes to files other than A).
For that you may need to do something like:

	git-diff-index --cached HEAD ;# already in index but do not look at A
        git-diff-index HEAD -- A ;# and path A is taken from working tree

which is a bit cumbersome.

Without --also (the new semantics), the check would be
straightforward:

	$ work on file A
        $ git diff A
        ... that looks fine so far ...
        $ git update-index A
        $ work more on file A
        $ git diff A
        ... incrementally that looks fine ...
 +++++  $ git diff HEAD -- A
	$ git commit A