git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Carl Worth <cworth@cworth.org>
To: Junio C Hamano <junkio@cox.net>
Cc: Andy Parkins <andyparkins@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] Make git-commit cleverer - have it figure out whether it needs -a automatically
Date: Sat, 02 Dec 2006 01:45:16 -0800	[thread overview]
Message-ID: <87slfy3a37.wl%cworth@cworth.org> (raw)
In-Reply-To: <7vbqmmzplm.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 3819 bytes --]

On Sat, 02 Dec 2006 00:09:09 -0800, Junio C Hamano wrote:
> But I think the problem with this 'cleverer' commit runs
> deeper.

I agree. Being too clever is a problem.

It's very helpful to estimate usability and learnability by the length
of prose needed to describe a command.

>  1. With paths, "git commit <paths>" means "forget for a moment
>     the changes I staged to be committed, and make a commit that
>     includes only these paths (i.e. the new commit and the
>     current HEAD are different at exactly these paths and
>     nowhere else, and the new commit has contents from the
>     working tree for these paths)".
>
>  2. Without paths, "git commit" means "make a commit out of
>     everything I have told you to commit (aka 'staged') so far".
>     The primary ways to tell git to stage contents are "git
>     add/rm/mv".  But as a short-hand, you can say "git commit
>     -a" to ask the command to place all the changes in the
>     working tree in the changeset to be committed before making
>     the new commit.

Junio, thanks so much for these descriptions. They help ground the
discussion quite nicely, (and will also contribute to improved
documentation).

Here's pseudo-code for the above descriptions:

	if (command-line has paths) {
		ignore staging area, commit named files
	else {
		if (commit -a)
			update all files into staging area
		commit staging area
	}

One problem I see in that is that the primary distinction is made
based on what appears on the command-line, rather than what job the
user is trying to perform. Also, "commit -a" is define in terms of the
staging area, even though the staging area is basically irrelevant to
this operation, (just as it is in the case of a commit with paths).

So I would re-factor that in a way that focuses on what the user is
trying to do:

	if (! doing a staged commit) {
		if (file list is empty)
			file list = all tracked files
		commit file list
	} else {
		commit staging area
	}

This brings the description of "commit -a" and "commit files..."
together, (which I think are conceptually more related than "commit
-a" is to a commit of the staging area, (and yes, this ignores the
history of the implementation). What we're talking about is how to
document what the user wants to do, not how the implementation does
it.

Notice also that "staging area" never appears in the description of
the else clause, (which is good since the conceptual use of these
commands does not involve staging).

So translating my re-factored version back into prose, we might get:

   commit <paths>
   commit -a

	Commit the working-tree contents of the named <paths>, (or all
	tracked paths for -a). Files which no longer exist in the
	working tree will be removed. New files to be tracked must be
	added with "git add".

   commit

	Commit the content that exists in the staging area. The
	staging area initially consists of the contents of the most
	recent commit, but can be modified with the "git add",
	"git rm", and "git mv".

So that's shorter. I think it's also more clear and focused on what
the user wants to do without being any less accurate.

It doesn't make it obvious that "commit -a" is the most common form
and what users should look at first. So what I'd like to see is the
semantic changes that would allow us to document this as:

   commit
   commit <paths>

	Commit the working-tree contents of all tracked paths, (or
	just the specific paths listed). Files which no longer exist
	in the working tree will be removed. New files to be tracked
	must be added with "git add".

   commit --staged

	Commit the content that exists in the staging area. The
	staging area initially consists of the contents of the most
	recent commit, but updated content from the working tree can
	be placed into it with "git stage <paths>".

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

  reply	other threads:[~2006-12-02  9:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-01 11:06 [PATCH] Make git-commit cleverer - have it figure out whether it needs -a automatically Andy Parkins
2006-12-01 11:15 ` Junio C Hamano
2006-12-01 11:32   ` Junio C Hamano
2006-12-01 12:33   ` Andy Parkins
2006-12-02  8:09 ` Junio C Hamano
2006-12-02  9:45   ` Carl Worth [this message]
2006-12-03  9:57   ` Andy Parkins
  -- strict thread matches above, loose matches on Subject: below --
2006-11-30 13:13 [PATCH/RFC] " Jakub Narebski
2006-11-30 13:24 ` [PATCH] " Andy Parkins
2006-11-30 13:32   ` Nguyen Thai Ngoc Duy
2006-11-30 13:41     ` Jakub Narebski
2006-11-30 15:01     ` Andy Parkins
2006-11-30 15:43       ` Salikh Zakirov
2006-11-30 16:28       ` Jakub Narebski
2006-12-01 10:59         ` Andy Parkins

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

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

  git send-email \
    --in-reply-to=87slfy3a37.wl%cworth@cworth.org \
    --to=cworth@cworth.org \
    --cc=andyparkins@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /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.
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 NNTP newsgroup(s).