git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <junkio@cox.net>
To: Carl Worth <cworth@cworth.org>
Cc: git@vger.kernel.org, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [RFC] Two conceptually distinct commit commands
Date: Wed, 06 Dec 2006 10:31:21 -0800	[thread overview]
Message-ID: <7virgo511i.fsf@assigned-by-dhcp.cox.net> (raw)
In-Reply-To: 874ps91v79.wl%cworth@cworth.org

Carl Worth <cworth@cworth.org> writes:

> ... Even if this functionality
> weren't made available at all, I'd still be interested in your
> comments on the main thrust of my proposal. I think that consists of:
>
> 	1. Unifying the two current commands that provide
> 	   commit-working-tree-content semantics into a single,
> 	   use-oriented description.
>
> 	2. Avoiding a change of semantics triggered by merely applying
> 	   pathname arguments without any command-line option or
> 	   alternate command name.

I am not sure what needs to be commented on at this point, since
it is not yet clear to me where you want your proposal to lead
us.

I do not agree with your "three commands" or "two semantics"
characterization of the current way "git commit" works.  "git
commit" without any optional argument already acts as if a
sensible default arguments are given, that is "no funny business
with additional paths, commit just what the user has staged
already."

"git commit" is primarily about committing what has been staged
in the index, and "--all" is just a type-saver short-hand (just
like "--include" is) to perform update-index the last minute and
nothing more.  In other words, "--all" is a variant of the
pathname-less form "git commit".  It is not a variant of "git
commit --only paths..." form, as you characterized.

The pathname form (the "--only" variant) on the surface seem to
work differently, but when you think about it, it is not all
that different from the normal commit.  We explain that it
ignores index, but in the bigger picture, it does not really.

In this sequence:

	edit a b
	git update-index a
	git commit --only b
	git commit --all

the first commit does "jump" the changes already made to the
index, but after it makes the commit, the index has the same
contents as if you did "git update-index a b" where you ran that
"git commit".  In other words, it is just a handy short-hand to
pretend as if you did the above sequence in this order instead:

	edit a b
        git update-index b
        git commit
        git update-index a
        git commit

So I actually think it is a mistake to stress the fact that "git
commit --only paths..." seems to act differently from the normal
"git commit" too much.  It just helps to split the changes in
your working tree if the changes happen to be cleanly separable
at file boundaries (aka "CVS mentality").  When the changes are
not cleanly separable at file boundaries, the "more painfully
index aware" variant also allows you to split the changes in
your working tree in the time dimension:

        edit a
	git update-index a
        edit a
        git commit ;# without paths
	git update-index a
        git commit

In short, while I understand that your "proposal" shows your own
way to summarize the semantics of "git commit", I am not seeing
what it buys us, and I do not see the need to come up with a
pair of new two commands for making commits (if that is what the
proposal is about, that is, but it is not clear to me if that is
what you are driving at).  I think it would only confuse users.

> 	I receive a patch while I'm in the middle of doing other work,
> 	(but with a clean index compared to HEAD, which is what I've
> 	usually). The patch looks good, so I want to commit it right
> 	away, but I do want to separate it into two or more pieces,
> 	(commonly this is because I want to separate the "add a test
> 	case demonstrating a bug" part from the "fix the bug"
> 	part).
> ...
> Who said I wouldn't test it? I do split commits like this precisely so
> that I _can_ test it this way---and git helps a lot here. I do the
> split commit, then easily back up to the revision that adds the test
> case, verify the test fails before the bug fix, (which is something
> the maintainer doesn't get a chance to do with your (2) approach),
> then move forward and verify that the test passes after the fix.
>
> So, sure, I haven't ever had that working tree before the commit. But
> git makes it easy to get that working tree after I commit and test
> everything before I push anything out.

You saw a good patch in the middle of something that you did not
want to lose your working tree changes for.  That good patch was
not really good enough to be applied straight into your tree but
needed tweaking and splitting.  Nevertheless you went ahead and
made two commits out of that patch, even though you were in the
middle of something.  You could not test them right away after
committing because your tree was in no shape to test them in
isolation.  But that is excusable because you would not push
these commits out right away, before you have a chance to test
them by rewinding your working tree when you are done with what
you were originally doing.

Is it just me who finds the above a very much made-up example?

It means the patch (which is good and not good at the same time)
was not all that urgent after all, and it could well have waited
until you are done with what you were originally doing.

In any case, I should clarify my aversion to partial commits a
bit.  What is more important is to notice that, while you cannot
compile-and-run test what is in the index in isolation (without
a fuse that exports the index contents as a virtual filesystem
-- anybody interested?), you _can_ preview and verify the text
that is going to be committed by comparing the index and the
HEAD.  And for that, your "staging" action (i.e. Nico's "git
add") needs to be a separate step from your "committing" action.

In other words, I would even love Johannes's "per hunk commit"
idea, at least if it had an option to preview the whole thing
just one more time before committing, and I would love it better
if it had an option for not committing but just updating.  You
could:

	$ edit foo bar
	: the whole mess in working tree is in no shape to be committed.
        $ git add foo	;# stage the state of the entire file
        $ git hunk-add bar ;# go interactive and update index selectively
	$ git status -v	;# that is "git commit --dry-run --diff"

to review what would be committed.  So while the commit that
would be made may not be compile-and-run tested, I would not
mind partial commit that much (and after all not all the
projects that track their contents with git are not "compiled"
nor "need testing" projects -- they could be tracking plain text
documentation, and the last-minute eyeballing may be a good
enough test for such contents).

  parent reply	other threads:[~2006-12-06 18:31 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-04 19:08 [RFC] Two conceptually distinct commit commands Carl Worth
2006-12-04 20:10 ` Carl Worth
2006-12-04 21:19   ` Jakub Narebski
2006-12-05  2:36     ` Carl Worth
2006-12-05  0:52 ` Horst H. von Brand
2006-12-05  1:18   ` Carl Worth
2006-12-05  2:14     ` Horst H. von Brand
2006-12-05  2:32       ` Carl Worth
2006-12-05  1:19   ` Jakub Narebski
2006-12-05  3:51 ` Theodore Tso
2006-12-05  6:33   ` Junio C Hamano
2006-12-05  6:38   ` Carl Worth
2006-12-06  1:13 ` Junio C Hamano
2006-12-06  4:53   ` Carl Worth
2006-12-06  9:54     ` Commit order in git.git, was " Johannes Schindelin
2006-12-06 16:14     ` Carl Worth
2006-12-06 18:31     ` Junio C Hamano [this message]
2006-12-06 23:29       ` Carl Worth

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7virgo511i.fsf@assigned-by-dhcp.cox.net \
    --to=junkio@cox.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=cworth@cworth.org \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).