Re: Two ideas for improving git's user interface

Carl Worth <cworth@cworth.org> writes:

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.

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