[PATCH] git-notes.txt: clarify -C vs. copy and -F

[PATCH] git-notes.txt: clarify -C vs. copy and -F

[Michael J Gruber]

The current description of '-C' together with the analogy to 'git commit
-C' can lead to the wrong conclusion that '-C' copies notes between
objects. Make this clearer by rewording and pointing to 'copy'.

The example for attaching binary notes with 'git hash-object' followed
by 'git notes add -C' immediately raises the question: "Why not use 'git
notes add -F'?". Answer it (the latter is not binary-safe).

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
In fact, the long name '--reuse-message' is really misleading, but I've been
around long enough to refrain from trying to change it ;)

 Documentation/git-notes.txt |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 296f314..c63b593 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -138,8 +138,9 @@ OPTIONS
 
 -C <object>::
 --reuse-message=<object>::
-	Take the note message from the given blob object (for
-	example, another note).
+	Take the given blob object (for	example, another note) as the
+	note message. (Use `git notes copy <object>` instead to
+	copy notes between objects.) 
 
 -c <object>::
 --reedit-message=<object>::
@@ -272,6 +273,8 @@ $ blob=$(git hash-object -w a.out)
 $ git notes --ref=built add -C "$blob" HEAD
 ------------
 
+(You cannot simply use `git notes --ref=built add -F a.out HEAD`
+because that is not binary-safe.)
 Of course, it doesn't make much sense to display non-text-format notes
 with 'git log', so if you use such notes, you'll probably need to write
 some special-purpose tools to do something useful with them.
-- 
1.7.4.1.607.g888da

Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F

[Johan Herland]

On Tuesday 29 March 2011, Michael J Gruber wrote:
> The current description of '-C' together with the analogy to 'git
> commit -C' can lead to the wrong conclusion that '-C' copies notes
> between objects. Make this clearer by rewording and pointing to
> 'copy'.
>
> The example for attaching binary notes with 'git hash-object'
> followed by 'git notes add -C' immediately raises the question: "Why
> not use 'git notes add -F'?". Answer it (the latter is not
> binary-safe).
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>

Acked-by: Johan Herland <johan@herland.net>


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> The current description of '-C' together with the analogy to 'git commit
> -C' can lead to the wrong conclusion that '-C' copies notes between
> objects. Make this clearer by rewording and pointing to 'copy'.
>
> The example for attaching binary notes with 'git hash-object' followed
> by 'git notes add -C' immediately raises the question: "Why not use 'git
> notes add -F'?". Answer it (the latter is not binary-safe).
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> In fact, the long name '--reuse-message' is really misleading, but I've been
> around long enough to refrain from trying to change it ;)

Yeah, it utterly is broken.  Why not fix it before people start making
serious use of notes?

>  Documentation/git-notes.txt |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index 296f314..c63b593 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -138,8 +138,9 @@ OPTIONS
>  
>  -C <object>::
>  --reuse-message=<object>::
> -	Take the note message from the given blob object (for
> -	example, another note).
> +	Take the given blob object (for	example, another note) as the
> +	note message. (Use `git notes copy <object>` instead to
> +	copy notes between objects.) 
>  
>  -c <object>::
>  --reedit-message=<object>::
> @@ -272,6 +273,8 @@ $ blob=$(git hash-object -w a.out)
>  $ git notes --ref=built add -C "$blob" HEAD
>  ------------
>  
> +(You cannot simply use `git notes --ref=built add -F a.out HEAD`
> +because that is not binary-safe.)
>  Of course, it doesn't make much sense to display non-text-format notes
>  with 'git log', so if you use such notes, you'll probably need to write
>  some special-purpose tools to do something useful with them.

Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> The current description of '-C' together with the analogy to 'git commit
>> -C' can lead to the wrong conclusion that '-C' copies notes between
>> objects. Make this clearer by rewording and pointing to 'copy'.
>>
>> The example for attaching binary notes with 'git hash-object' followed
>> by 'git notes add -C' immediately raises the question: "Why not use 'git
>> notes add -F'?". Answer it (the latter is not binary-safe).
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>> In fact, the long name '--reuse-message' is really misleading, but I've been
>> around long enough to refrain from trying to change it ;)
>
> Yeah, it utterly is broken.  Why not fix it before people start making
> serious use of notes?

Actually I take it back and throw it again after doubling it.  Not just
the long name, but using -C/-c is already utterly broken.  These are meant
to reuse (meta)data associated with an existing object, not using some
data that happens to be stored in a random loose blob.  I don't think of
any similar option anywhere in git.

Instead of mucking with the documentation, why not fix the behaviour to
match what -C/-c/--reuse usually means, which is what the documentation
describes?

Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 29.03.2011 20:25:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>
>>> The current description of '-C' together with the analogy to 'git commit
>>> -C' can lead to the wrong conclusion that '-C' copies notes between
>>> objects. Make this clearer by rewording and pointing to 'copy'.
>>>
>>> The example for attaching binary notes with 'git hash-object' followed
>>> by 'git notes add -C' immediately raises the question: "Why not use 'git
>>> notes add -F'?". Answer it (the latter is not binary-safe).
>>>
>>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>>> ---
>>> In fact, the long name '--reuse-message' is really misleading, but I've been
>>> around long enough to refrain from trying to change it ;)
>>
>> Yeah, it utterly is broken.  Why not fix it before people start making
>> serious use of notes?

You seriously ask why? Because I've banged my head way to often by
suggesting behavior changes!

> 
> Actually I take it back and throw it again after doubling it.  Not just
> the long name, but using -C/-c is already utterly broken.  These are meant
> to reuse (meta)data associated with an existing object, not using some
> data that happens to be stored in a random loose blob.  I don't think of
> any similar option anywhere in git.
> 
> Instead of mucking with the documentation, why not fix the behaviour to
> match what -C/-c/--reuse usually means, which is what the documentation
> describes?

Because it's not what the doc describes. The current version is easy to
misunderstand, but in connection with the example it is clear how it is
meant, and that's how it is implemented. If I were to reimplement it I
would:

- make "notes add -C/-c" really analogous to "commit -c/-C", i.e. do
"notes copy"

- make -F binary safe

and while at it rename "add" to "edit", because I've been bitten too
often by trying to add to a note using the "add" command. But all these
are behavior changes/incompatibilities, i.e. a no-go.

I don't mean to criticize the initial implementation of "notes", it just
shows that we detect rough ui edges only after using a feature. I'm all
for changes, I just rarely can get myself to making a hopeless feature
change patch any more.

Michael

Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F

[Junio C Hamano]

Michael J Gruber <drmicha@warpmail.net> writes:

>>> Yeah, it utterly is broken.  Why not fix it before people start making
>>> serious use of notes?
>
> You seriously ask why? Because I've banged my head way to often by
> suggesting behavior changes!

Change of a behaviour that nobody had chance to have seriously used for
only 6 weeks or so?  I'd call that a fair game.

> If I were to reimplement it I would:
>
> - make "notes add -C/-c" really analogous to "commit -c/-C", i.e. do
> "notes copy"

That is sensible.

> - make -F binary safe

Likewise.

> and while at it rename "add" to "edit"

That one I think is older wart that may be harder to change.

Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Michael J Gruber <drmicha@warpmail.net> writes:
>
>>>> Yeah, it utterly is broken.  Why not fix it before people start making
>>>> serious use of notes?
>>
>> You seriously ask why? Because I've banged my head way to often by
>> suggesting behavior changes!
>
> Change of a behaviour that nobody had chance to have seriously used for
> only 6 weeks or so?  I'd call that a fair game.

Oops, sorry I misread the timestamp.  It has been one year and 6 weeks.
Hmm, but still -C/-c is wrong.

I am very strongly tempted to fix the semantics, though.

[PATCH] git-notes.txt: clarify -C vs. copy and -F

[Michael J Gruber]

The current description of '-C' together with the analogy to 'git commit
-C' can lead to the wrong conclusion that '-C' copies notes between
objects. Make this clearer by rewording and pointing to 'copy'.

The example for attaching binary notes with 'git hash-object' followed
by 'git notes add -C' immediately raises the question: "Why not use 'git
notes add -F'?". Answer it (the latter is not binary-safe).

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
This one has been lying around and fell under the rugs of the discussion
for a ui redesign which never happened. So I think it's still worth it.
---
 Documentation/git-notes.txt |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 6a187f2..e8319ea 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -142,8 +142,9 @@ OPTIONS
 
 -C <object>::
 --reuse-message=<object>::
-	Take the note message from the given blob object (for
-	example, another note).
+	Take the given blob object (for	example, another note) as the
+	note message. (Use `git notes copy <object>` instead to
+	copy notes between objects.)
 
 -c <object>::
 --reedit-message=<object>::
@@ -285,6 +286,8 @@ $ blob=$(git hash-object -w a.out)
 $ git notes --ref=built add -C "$blob" HEAD
 ------------
 
+(You cannot simply use `git notes --ref=built add -F a.out HEAD`
+because that is not binary-safe.)
 Of course, it doesn't make much sense to display non-text-format notes
 with 'git log', so if you use such notes, you'll probably need to write
 some special-purpose tools to do something useful with them.
-- 
1.7.6.845.gc3c05

Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F

[Johan Herland]

On Thursday 25 August 2011 12:26:37 Michael J Gruber wrote:
> The current description of '-C' together with the analogy to 'git commit
> -C' can lead to the wrong conclusion that '-C' copies notes between
> objects. Make this clearer by rewording and pointing to 'copy'.
> 
> The example for attaching binary notes with 'git hash-object' followed
> by 'git notes add -C' immediately raises the question: "Why not use 'git
> notes add -F'?". Answer it (the latter is not binary-safe).
> 
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> This one has been lying around and fell under the rugs of the discussion
> for a ui redesign which never happened. So I think it's still worth it.

ACK

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

[RFC/PATCH] Make "git notes add" more user-friendly when there are existing notes

[Johan Herland]

Currently, "notes add" (without -f/--force) will abort when the given object
already has existing notes. This makes sense for the modes of "git notes add"
that would necessarily overwrite the old message (when using the -m/-F/-C/-c
options). However, when no options are given (meaning the notes are created
from scratch in the editor) it is not very user-friendly to abort on existing
notes, and forcing the user to run "git notes edit".

Instead, it is better to simply "redirect" to "git notes edit" automatically,
i.e. open the existing notes in the editor and let the user edit them.
This patch does just that.

This changes the behavior of "git notes add" without options when notes
already exist for the given object, but I doubt that many users really depend
on the previous failure from "git notes add" in this case.

Signed-off-by: Johan Herland <johan@herland.net>
---

On Tuesday 29 March 2011, Junio C Hamano wrote:
> Michael J Gruber <drmicha@warpmail.net> writes:
> > and while at it rename "add" to "edit"
> That one I think is older wart that may be harder to change.

Here's one attempt at giving Michael a nicer "git notes add" without
breaking too many existing users. It's not very pretty, but I hope it
gets the job done without inconveniencing current users too much.

After all, current (script) users of "git notes add" that depend on it
failing to overwrite existing notes, should already use -m/-F/-C/-c
instead of the default interactive mode, anyway.


Have fun! :)

...Johan


 Documentation/git-notes.txt |    7 +++++--
 builtin/notes.c             |   19 ++++++++++++++++---
 t/t3301-notes.sh            |   29 +++++++++++++++++++++++++++--
 3 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 296f314..913ecd8 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -57,8 +57,11 @@ list::
 
 add::
 	Add notes for a given object (defaults to HEAD). Abort if the
-	object already has notes (use `-f` to overwrite an
-	existing note).
+	object already has notes (use `-f` to overwrite existing notes).
+	However, if you're using `add` interactively (using an editor
+	to supply the notes contents), then - instead of aborting -
+	the existing notes will be opened in the editor (like the `edit`
+	subcommand).
 
 copy::
 	Copy the notes for the first object onto the second object.
diff --git a/builtin/notes.c b/builtin/notes.c
index 0aab150..4074ba1 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -527,6 +527,8 @@ static int list(int argc, const char **argv, const char *prefix)
 	return retval;
 }
 
+static int append_edit(int argc, const char **argv, const char *prefix);
+
 static int add(int argc, const char **argv, const char *prefix)
 {
 	int retval = 0, force = 0;
@@ -554,14 +556,14 @@ static int add(int argc, const char **argv, const char *prefix)
 	};
 
 	argc = parse_options(argc, argv, prefix, options, git_notes_add_usage,
-			     0);
+			     PARSE_OPT_KEEP_ARGV0);
 
-	if (1 < argc) {
+	if (2 < argc) {
 		error("too many parameters");
 		usage_with_options(git_notes_add_usage, options);
 	}
 
-	object_ref = argc ? argv[0] : "HEAD";
+	object_ref = argc > 1 ? argv[1] : "HEAD";
 
 	if (get_sha1(object_ref, object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);
@@ -571,6 +573,17 @@ static int add(int argc, const char **argv, const char *prefix)
 
 	if (note) {
 		if (!force) {
+			if (!msg.given) /* redirect to "edit" subcommand */
+			{
+				/*
+				 * We only end up here if none of -m/-F/-c/-C
+				 * or -f are given. The original args are
+				 * therefore still in argv[0-1]
+				 */
+				argv[0] = "edit";
+				free_notes(t);
+				return append_edit(argc, argv, prefix);
+			}
 			retval = error("Cannot add notes. Found existing notes "
 				       "for object %s. Use '-f' to overwrite "
 				       "existing notes", sha1_to_hex(object));
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 1921ca3..3448c23 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -101,8 +101,8 @@ test_expect_success 'edit existing notes' '
 	test_must_fail git notes show HEAD^
 '
 
-test_expect_success 'cannot add note where one exists' '
-	! MSG=b2 git notes add &&
+test_expect_success 'cannot "git notes add -m" where notes already exists' '
+	test_must_fail git notes add -m "b2" &&
 	test ! -f .git/NOTES_EDITMSG &&
 	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
 	test b3 = $(git notes show) &&
@@ -110,6 +110,24 @@ test_expect_success 'cannot add note where one exists' '
 	test_must_fail git notes show HEAD^
 '
 
+test_expect_success 'can overwrite existing note with "git notes add -f -m"' '
+	git notes add -f -m "b1" &&
+	test ! -f .git/NOTES_EDITMSG &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b1 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
+test_expect_success 'add w/no options on existing note morphs into edit' '
+	MSG=b2 git notes add &&
+	test ! -f .git/NOTES_EDITMSG &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b2 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
 test_expect_success 'can overwrite existing note with "git notes add -f"' '
 	MSG=b1 git notes add -f &&
 	test ! -f .git/NOTES_EDITMSG &&
@@ -194,6 +212,13 @@ test_expect_success 'show -F notes' '
 	test_cmp expect-F output
 '
 
+test_expect_success 'Re-adding -F notes without -f fails' '
+	echo "zyxxy" > note5 &&
+	test_must_fail git notes add -F note5 &&
+	git log -3 > output &&
+	test_cmp expect-F output
+'
+
 cat >expect << EOF
 commit 15023535574ded8b1a89052b32673f84cf9582b8
 tree e070e3af51011e47b183c33adf9736736a525709
-- 
1.7.4

Re: [RFC/PATCH] Make "git notes add" more user-friendly when there are existing notes

[Michael J Gruber]

Johan Herland venit, vidit, dixit 30.03.2011 02:02:
> Currently, "notes add" (without -f/--force) will abort when the given object
> already has existing notes. This makes sense for the modes of "git notes add"
> that would necessarily overwrite the old message (when using the -m/-F/-C/-c
> options). However, when no options are given (meaning the notes are created
> from scratch in the editor) it is not very user-friendly to abort on existing
> notes, and forcing the user to run "git notes edit".
> 
> Instead, it is better to simply "redirect" to "git notes edit" automatically,
> i.e. open the existing notes in the editor and let the user edit them.
> This patch does just that.
> 
> This changes the behavior of "git notes add" without options when notes
> already exist for the given object, but I doubt that many users really depend
> on the previous failure from "git notes add" in this case.
> 
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
> 
> On Tuesday 29 March 2011, Junio C Hamano wrote:
>> Michael J Gruber <drmicha@warpmail.net> writes:
>>> and while at it rename "add" to "edit"
>> That one I think is older wart that may be harder to change.
> 
> Here's one attempt at giving Michael a nicer "git notes add" without
> breaking too many existing users. It's not very pretty, but I hope it
> gets the job done without inconveniencing current users too much.

That is certainly an improvement, though I'm still wondering how large a
change we're aiming at, given Junio's remarks. Things I would like to
throw in:

* options vs. arguments:

"tag", "branch" etc. use options for subcommands, e.g. "tag -d", "branch
-d" etc. "remote", "stash" use arguments, e.g. "remote add", "stash
list". I don't see us unifying that, but we should decide about a
direction to go for "new" commands and stick to that. I feel that
options are the way to go. What I really feel strongly about is that we
should decide once and then stick to that for future commands (and may
be gradually revamping).

* singular vs. plural:

All our porcelain commands are singular even when they deal with
multiple items (tag, branch, remote, submodule, ...). "notes" is the
only exception, why not have it be "note"? (That would also open up a
migration strategy, though the usual suspects may not even bother ;))

* "notes message":

The term seems to be used to distinguish between the content of a note
and the note object (blob content vs. blob object). A regular git user
may think it is the commit message in the notes log, i.e.:

git log $(git notes get-ref)

I'm wondering whether we should actually expose those note commit
messages. If notes are shared then editing a note may require an
explanation just like other commits do, especially when they get used
for other things than "notes" in the proper sense.

If we do that, then -m,-c,-C etc. would need to be analogous to "git
commit -m,-c,-C", i.e. about note commit messages, not about the actual
note. If we completely discard the possibility that users will look at
the notes log and write note commit messages, we can use the "regular
commit message <-> notes content" analogy for the options that we
partially have now (and adjust -c,-C).

Cheers,
Michael

Re: [RFC/PATCH] Make "git notes add" more user-friendly when there are existing notes

[Johan Herland]

On Wednesday 30 March 2011, Michael J Gruber wrote:
> I'm still wondering how large a
> change we're aiming at, given Junio's remarks. Things I would like to
> throw in:
> 
> * options vs. arguments:
> 
> "tag", "branch" etc. use options for subcommands, e.g. "tag -d", "branch
> -d" etc. "remote", "stash" use arguments, e.g. "remote add", "stash
> list". I don't see us unifying that, but we should decide about a
> direction to go for "new" commands and stick to that. I feel that
> options are the way to go. What I really feel strongly about is that we
> should decide once and then stick to that for future commands (and may
> be gradually revamping).

This is a big discussion, and I don't really have a strong opinion either 
way (or on whether unification of options vs. arguments is really necessary 
at all). In general, I like separating the "verb" of the command (_what_ to 
do) from the "adverbs" (_how_ to do it). For some git commands, the verb is 
right there in the name (e.g. "checkout", "add", "rm", etc.), so the options 
are usually all "adverbs". Other commands, however, refer to one of git's 
"subsystems" (for some very vague definition of "subsystem") as a "noun" 
(e.g. "stash", "remote", "notes"), and the verb needs to be specified 
(either as a subcommand, or as an option). In those cases, I personally 
prefer the subcommand approach ("git noun verb --adverb") better than the 
option approach ("git noun --verb --adverb"), so as to separate the verb 
from the adverbs.

However, some commands (e.g. "branch", "tag") are _both_ "verbs" ("I want to 
tag something") and "nouns" ("I want to add a tag"). By now, I'm thoroughly 
used to "branch -d" and "tag -d", so e.g. "branch rm" and "tag rm" look a 
bit foreign to me, although they probably follow the above principle more 
closely...

Then you have weird cases that further complicate things: "rebase" is 
usually a verb, but in "rebase --continue" or "rebase --abort" another verb 
takes the focus, and I would probably prefer them as subcommands ("rebase 
continue" and "rebase abort").

What can I say? Habits are hard to break, and this might be a case where 
breaking them is more harmful than a somewhat messy command-line interface.

> * singular vs. plural:
> 
> All our porcelain commands are singular even when they deal with
> multiple items (tag, branch, remote, submodule, ...). "notes" is the
> only exception, why not have it be "note"? (That would also open up a
> migration strategy, though the usual suspects may not even bother ;))

True, but would you want to use "note" as a verb or a noun?

  Verb:
  $ git note # to add/edit a note
  $ git note -d # to remove a note
  etc.

  Noun:
  $ git note add # to add/edit a note
  $ git note rm # to remove a note
  etc.

> * "notes message":
> 
> The term seems to be used to distinguish between the content of a note
> and the note object (blob content vs. blob object). A regular git user
> may think it is the commit message in the notes log, i.e.:
> 
> git log $(git notes get-ref)
> 
> I'm wondering whether we should actually expose those note commit
> messages. If notes are shared then editing a note may require an
> explanation just like other commits do, especially when they get used
> for other things than "notes" in the proper sense.
> 
> If we do that, then -m,-c,-C etc. would need to be analogous to "git
> commit -m,-c,-C", i.e. about note commit messages, not about the actual
> note. If we completely discard the possibility that users will look at
> the notes log and write note commit messages, we can use the "regular
> commit message <-> notes content" analogy for the options that we
> partially have now (and adjust -c,-C).

Interesting. Originally, I think "notes message" comes from the initial use 
case of notes being an extension of the commit message. The "notes message" 
is therefore what is shown next to the commit message, i.e. the blob 
content. From that POV, the --reuse-message/--reedit-message options also 
make some sense.

But it is apparent by now that simply extending the commit message will 
probably not be the central use case for notes, and I agree that it makes 
sense to revisit the terminology (both in the documentation and the options 
themselves.)

As you say, -m/-c/-C should probably change to affect the commit message of 
the note commit (and not affect the note content). I'm unsure whether -F 
should follow along, or if we should reserve that for supplying note content 
(binary-safely). I think I prefer the former, and would want a different 
option for getting note contents from a file.

Obviously, copying notes from one object to another is covered by "git notes 
copy", but I wonder if it still makes sense to provide a way to get note 
contents from an existing blob SHA1 (i.e. what -c/-C does today).


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [RFC/PATCH] Make "git notes add" more user-friendly when there are existing notes

[Lasse Makholm]

On 30 March 2011 11:59, Johan Herland <johan@herland.net> wrote:
>> * options vs. arguments:
>>
>> "tag", "branch" etc. use options for subcommands, e.g. "tag -d", "branch
>> -d" etc. "remote", "stash" use arguments, e.g. "remote add", "stash
>> list". I don't see us unifying that, but we should decide about a
>> direction to go for "new" commands and stick to that. I feel that
>> options are the way to go. What I really feel strongly about is that we
>> should decide once and then stick to that for future commands (and may
>> be gradually revamping).
>
> This is a big discussion, and I don't really have a strong opinion either
> way (or on whether unification of options vs. arguments is really necessary
> at all). In general, I like separating the "verb" of the command (_what_ to
> do) from the "adverbs" (_how_ to do it). For some git commands, the verb is
> right there in the name (e.g. "checkout", "add", "rm", etc.), so the options
> are usually all "adverbs". Other commands, however, refer to one of git's
> "subsystems" (for some very vague definition of "subsystem") as a "noun"
> (e.g. "stash", "remote", "notes"), and the verb needs to be specified
> (either as a subcommand, or as an option). In those cases, I personally
> prefer the subcommand approach ("git noun verb --adverb") better than the
> option approach ("git noun --verb --adverb"), so as to separate the verb
> from the adverbs.
>
> However, some commands (e.g. "branch", "tag") are _both_ "verbs" ("I want to
> tag something") and "nouns" ("I want to add a tag"). By now, I'm thoroughly
> used to "branch -d" and "tag -d", so e.g. "branch rm" and "tag rm" look a
> bit foreign to me, although they probably follow the above principle more
> closely...

Think of it less as the (only) verb and more of it as a domain. In the
domain of a git remote, (add|rm|rename|...) is the action (verb) and
that's why it is and should be a sub-command.

git remote and git stash do it right in my opinion. The default action
differs (list vs. create) but that's OK because so does the most
common use case.

The canonical way to create a stash is to say "git stash create" but
we allow you to simply say "git stash" because that's probably what
you want. It seems then, that the canonical way to create a commit
would be by saying "git commit create" (again, allowing the "git
commit" shortcut).

We could even expand on the heresy and argue that git log should be an
alias for "git commit list"... :-)

My fingers type git branch -d foo by habit as well, but were it to
change, I'd get over it and form new habits. We shouldn't let the
force of mere habits prevent us from doing The Right Thing.

You could argue that git branch -d is broken because -d is, in fact,
not an option at all. If it was, you would be able to say git branch
-d junk feature master to delete junk and branch out feature from
master. But you can't because -d really is a sub-command in disguise.

> Then you have weird cases that further complicate things: "rebase" is
> usually a verb, but in "rebase --continue" or "rebase --abort" another verb
> takes the focus, and I would probably prefer them as subcommands ("rebase
> continue" and "rebase abort").

Absolutely, yes. I don't see this as a weird case at all. In my view,
this is clearly broken just as git branch -d is. Again, in the domain
of a rebase, abort and continue are clearly commands and should loose
the dashes.

> What can I say? Habits are hard to break, and this might be a case where
> breaking them is more harmful than a somewhat messy command-line interface.

As someone, standing on the edge of a 1000+ developer deployment of
git, the option-vs-sub-command issue is one of the many things
currently keeping me up at night. I would take a break in habits any
day to avoid a lifetime of pain teaching people to remember and accept
these inconsistencies...

/Lasse

Re: [RFC/PATCH] Make "git notes add" more user-friendly when there are existing notes

[Michael J Gruber]

Lasse Makholm venit, vidit, dixit 04.04.2011 13:35:
> On 30 March 2011 11:59, Johan Herland <johan@herland.net> wrote:
>>> * options vs. arguments:
>>>
>>> "tag", "branch" etc. use options for subcommands, e.g. "tag -d", "branch
>>> -d" etc. "remote", "stash" use arguments, e.g. "remote add", "stash
>>> list". I don't see us unifying that, but we should decide about a
>>> direction to go for "new" commands and stick to that. I feel that
>>> options are the way to go. What I really feel strongly about is that we
>>> should decide once and then stick to that for future commands (and may
>>> be gradually revamping).
>>
>> This is a big discussion, and I don't really have a strong opinion either
>> way (or on whether unification of options vs. arguments is really necessary
>> at all). In general, I like separating the "verb" of the command (_what_ to
>> do) from the "adverbs" (_how_ to do it). For some git commands, the verb is
>> right there in the name (e.g. "checkout", "add", "rm", etc.), so the options
>> are usually all "adverbs". Other commands, however, refer to one of git's
>> "subsystems" (for some very vague definition of "subsystem") as a "noun"
>> (e.g. "stash", "remote", "notes"), and the verb needs to be specified
>> (either as a subcommand, or as an option). In those cases, I personally
>> prefer the subcommand approach ("git noun verb --adverb") better than the
>> option approach ("git noun --verb --adverb"), so as to separate the verb
>> from the adverbs.
>>
>> However, some commands (e.g. "branch", "tag") are _both_ "verbs" ("I want to
>> tag something") and "nouns" ("I want to add a tag"). By now, I'm thoroughly
>> used to "branch -d" and "tag -d", so e.g. "branch rm" and "tag rm" look a
>> bit foreign to me, although they probably follow the above principle more
>> closely...
> 
> Think of it less as the (only) verb and more of it as a domain. In the
> domain of a git remote, (add|rm|rename|...) is the action (verb) and
> that's why it is and should be a sub-command.
> 
> git remote and git stash do it right in my opinion. The default action
> differs (list vs. create) but that's OK because so does the most
> common use case.
> 
> The canonical way to create a stash is to say "git stash create" but
> we allow you to simply say "git stash" because that's probably what
> you want. It seems then, that the canonical way to create a commit
> would be by saying "git commit create" (again, allowing the "git
> commit" shortcut).
> 
> We could even expand on the heresy and argue that git log should be an
> alias for "git commit list"... :-)
> 
> My fingers type git branch -d foo by habit as well, but were it to
> change, I'd get over it and form new habits. We shouldn't let the
> force of mere habits prevent us from doing The Right Thing.
> 
> You could argue that git branch -d is broken because -d is, in fact,
> not an option at all. If it was, you would be able to say git branch
> -d junk feature master to delete junk and branch out feature from
> master. But you can't because -d really is a sub-command in disguise.
> 
>> Then you have weird cases that further complicate things: "rebase" is
>> usually a verb, but in "rebase --continue" or "rebase --abort" another verb
>> takes the focus, and I would probably prefer them as subcommands ("rebase
>> continue" and "rebase abort").
> 
> Absolutely, yes. I don't see this as a weird case at all. In my view,
> this is clearly broken just as git branch -d is. Again, in the domain
> of a rebase, abort and continue are clearly commands and should loose
> the dashes.
> 
>> What can I say? Habits are hard to break, and this might be a case where
>> breaking them is more harmful than a somewhat messy command-line interface.
> 
> As someone, standing on the edge of a 1000+ developer deployment of
> git, the option-vs-sub-command issue is one of the many things
> currently keeping me up at night. I would take a break in habits any
> day to avoid a lifetime of pain teaching people to remember and accept
> these inconsistencies...
> 

Well, I would like say that we should take this up as a long running
task then. The problem is, though, disambiguating things like "git
branch list" if we were to go for subcommands as arguments (not
options). I have no idea how to solve this (without having a complete
switch-over day).

Michael

Re: [RFC/PATCH] Make "git notes add" more user-friendly when there are existing notes

[Junio C Hamano]

Johan Herland <johan@herland.net> writes:

> Currently, "notes add" (without -f/--force) will abort when the given object
> already has existing notes. This makes sense for the modes of "git notes add"
> that would necessarily overwrite the old message (when using the -m/-F/-C/-c
> options). However, when no options are given (meaning the notes are created
> from scratch in the editor) it is not very user-friendly to abort on existing
> notes, and forcing the user to run "git notes edit".
>
> Instead, it is better to simply "redirect" to "git notes edit" automatically,
> i.e. open the existing notes in the editor and let the user edit them.
> This patch does just that.
>
> This changes the behavior of "git notes add" without options when notes
> already exist for the given object, but I doubt that many users really depend
> on the previous failure from "git notes add" in this case.
>
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
>
> On Tuesday 29 March 2011, Junio C Hamano wrote:
>> Michael J Gruber <drmicha@warpmail.net> writes:
>> > and while at it rename "add" to "edit"
>> That one I think is older wart that may be harder to change.
>
> Here's one attempt at giving Michael a nicer "git notes add" without
> breaking too many existing users. It's not very pretty, but I hope it
> gets the job done without inconveniencing current users too much.
>
> After all, current (script) users of "git notes add" that depend on it
> failing to overwrite existing notes, should already use -m/-F/-C/-c
> instead of the default interactive mode, anyway.

Looks sensible, by addressing the issue gently without going overboard.

Thanks; I like it.