Johan Herland <johan@xxxxxxxxxxx> writes:
> The following is a re-roll and resend of the patch series currently
> in pu, plus my own 2 patches for adding support for "-m" and "-F" to
> "git notes edit".
>
> On advice from Dscho, I have squashed the current bugfix and cleanup
> patches in js/notes into the first 4 "main" patches. As a result the
> original 15 + 2 patch series is now down to 5 (4 + 1) patches.
>
> In sum, these 5 patches produce the exact same result as the original
> js/notes series (plus my 2 patches).
Thanks, but you did no such thing, actually.
The result from this sequence
git checkout -b jh/notes v1.6.3.1^0
git am ./+jh-notes.mbox ;# these patches
git checkout v1.6.3.1^0
git merge js/notes ;# allow rerere to reapply the resolution
git diff -R jh/notes^
should be empty, or should show your improvements over what has been
queued in 'pu'.
Here is what I see:
diff --git b/git-notes.sh a/git-notes.sh
index 6ec33c9..7c3b8b9 100755
--- b/git-notes.sh
+++ a/git-notes.sh
@@ -20,15 +20,16 @@ edit)
die "Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)"
fi
- MESSAGE="$GIT_DIR"/new-notes-$COMMIT
+ MSG_FILE="$GIT_DIR/new-notes-$COMMIT"
+ GIT_INDEX_FILE="MSG_FILE.idx"
Renaming of the variable MESSAGE to MSG_FILE may be an improvement, as the
former could have puzzled the reader if $MESSAGE contains the message
itself, or it has the name of a file that stores the message (the answer
is latter).
But I think there is a regression with a missing '$' here.
+ export GIT_INDEX_FILE
+
trap '
- test -f "$MESSAGE" && rm "$MESSAGE"
+ test -f "$MSG_FILE" && rm "$MSG_FILE"
+ test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
' 0
Another improvement is that the variable definitions were moved up before
the trap that uses these definitions. This is a good change.
- GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
-
- GIT_INDEX_FILE="$MESSAGE".idx
- export GIT_INDEX_FILE
+ git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
Dscho's version exports an empty GIT_NOTES_REF, presumably to avoid
triggering the notes mechanism, while running this "git log -1", but yours
don't. Is there a reason behind this change?
The rest are fallouts from s/MESSAGE/MSG_FILE/ renaming.
@@ -36,16 +37,16 @@ edit)
else
PARENT="-p $CURRENT_HEAD"
git read-tree "$GIT_NOTES_REF" || die "Could not read index"
- git cat-file blob :$COMMIT >> "$MESSAGE" 2> /dev/null
+ git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
fi
core_editor="$(git config core.editor)"
- ${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MESSAGE"
+ ${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
- grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed
- mv "$MESSAGE".processed "$MESSAGE"
- if [ -s "$MESSAGE" ]; then
- BLOB=$(git hash-object -w "$MESSAGE") ||
+ grep -v ^# < "$MSG_FILE" | git stripspace > "$MSG_FILE".processed
+ mv "$MSG_FILE".processed "$MSG_FILE"
+ if [ -s "$MSG_FILE" ]; then
+ BLOB=$(git hash-object -w "$MSG_FILE") ||
die "Could not write into object database"
git update-index --add --cacheinfo 0644 $BLOB $COMMIT ||
die "Could not write index"
I am a bit reluctant to take this to replace js/notes as-is until I hear
answers to these points (and others may have comments too).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html