Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > This script allows you to edit and show commit notes easily. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> > --- > .gitignore | 1 + > Makefile | 2 +- > git-notes.sh | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 63 insertions(+), 1 deletions(-) > create mode 100755 git-notes.sh > > diff --git a/git-notes.sh b/git-notes.sh > new file mode 100755 > index 0000000..e0ad0b9 > --- /dev/null > +++ b/git-notes.sh > @@ -0,0 +1,61 @@ > +#!/bin/sh > + > +USAGE="(edit | show) [commit]" > +. git-sh-setup > + > +test -n "$3" && usage > + > +test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)" > +test -z "$GIT_NOTES_REF" && > + die "No notes ref set." test -n "${GIT_NOTES_REF=$(git config core.notesref)}" || die > +COMMIT=$(git rev-parse --verify --default HEAD "$2") This silently annotates the HEAD commit if $2 is misspelled, I suspect. Also if HEAD does not exist, COMMIT will be empty and this whole command will exit with non-zero status, which you would want to catch here... > +NAME=$(echo $COMMIT | sed "s/^../&\//") ... or here. > +case "$1" in > +edit) > + MESSAGE="$GIT_DIR"/new-notes > + GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE" $MESSAGE and its associated temporary file needs to be cleaned up upon command exit; perhaps a trap is in order. > + GIT_INDEX_FILE="$MESSAGE".idx > + export GIT_INDEX_FILE > + > + CURRENT_HEAD=$(git show-ref $GIT_NOTES_REF | cut -f 1 -d ' ') > + if [ -z "$CURRENT_HEAD" ]; then > + PARENT= > + else > + PARENT="-p $OLDTIP" > + git read-tree $GIT_NOTES_REF || die "Could not read index" > + git cat-file blob :$NAME >> "$MESSAGE" 2> /dev/null > + fi I take that OLDTIP is a typo. if CURRENT_HEAD=$(git show-ref -s "$GIT_NOTES_REF") then PARENT="-p $CURRENT_HEAD" ... else PARENT= fi > + > + ${VISUAL:-${EDITOR:-vi}} "$MESSAGE" > + > + grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed Makes us wonder if we would want to teach hash-stripping to git-stripspace, doesn't it? > + mv "$MESSAGE".processed "$MESSAGE" > + if [ -z "$(cat "$MESSAGE")" ]; then Make this 'if test -s "$MESSAGE"' and swap then/else clause around; no reason to slurp the value into your shell. > + test -z "$CURRENT_HEAD" && > + die "Will not initialise with empty tree" > + git update-index --force-remove $NAME || > + die "Could not update index" > + else > + BLOB=$(git hash-object -w "$MESSAGE") || > + die "Could not write into object database" > + git update-index --add --cacheinfo 0644 $BLOB $NAME || > + die "Could not write index" > + fi > + > + TREE=$(git write-tree) || die "Could not write tree" > + NEW_HEAD=$(: | git commit-tree $TREE $PARENT) || > + die "Could not annotate" Hmph. How about "echo Annotate $COMMIT | git commit-tree..."? > + case "$CURRENT_HEAD" in > + '') git update-ref $GIT_NOTES_REF $NEW_HEAD ;; > + *) git update-ref $GIT_NOTES_REF $NEW_HEAD $CURRENT_HEAD;; > + esac > +;; There are some places that have "$GIT_NOTES_REF" in dq and some places you don't. I think GIT_NOTES_REF begins with refs/ and consists only of valid refname characters, so unless the user wants to shoot himself in the foot it should be Ok, but we probably would want to quote it. Also, as unquoted $CURRENT_HEAD will not even count as a parameter to update-ref, you do not have to do that case/esac, but simply do: git update-ref "$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD Would we have reflog for this ref? What would we want to see as the message if we do? - 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