Re: [PATCH] Add git-save script

> Nanako Shiraishi <nanako3@bluebottle.com> writes:
>
>> +function save_work () {
>
> No noiseword "function" for portability.
>
> When you say "#!/bin/sh", you are writing for the family of
> generic Bourne shells, not specifically for korn nor bash.  For
> example, dash is a fine POSIX shell, but does not grok function
> noiseword.  When in doubt, please stay away from things not in
> POSIX (e.g. function, [[, ]], ${parameter//pattern/string/}).

>  - $GIT_DIR could contain shell metachararcters / whitespace, so
>    could $TMP as well.  Always quote such variables, or you risk
>    a surprise from "rm".
>
>  - You probably would not want to create a new "save" if your
>    working tree and the index are clean.  To test for the
>    condition, you can do something like:
>
> 	git-diff --quiet --cached && git-diff --quiet

>  - Although you keep a separate tree for the index (before the
>    "git add -u" to grab the working tree changes) in the saved
>    data, it does not seem to be used.  It _might_ make sense to
>    replace "git add -u" with "git add ." so that work/ tree
>    contains even untracked (but not ignored) files, and on the
>    restore side unstage the paths that appear in work/ but not
>    in indx/.  I dunno.

>> +
>> +	head=$(git-log --abbrev-commit --pretty=oneline -n 1 HEAD)
>> +	if branch=$(git symbolic-ref -q HEAD); then
>> +		branch=${branch#refs/heads/}
>> +	else
>> +		branch='(no branch)'
>> +	fi &&
>
> Minor style.  Please don't write "; then\n".  Line-up "then",
> "elif", "else", and "fi"; it is much easier to read that way.

> Nice, but after trying this myself a bit, I seriously wished for
> "git save show -p", so I did it myself, like this:
>
> 	show_save () {
> 		flags=$(git rev-parse --no-revs --flags "$@")
> 		if test -z "$flags"
> 		then
> 			flags=--stat
> 		fi
> 		save=$(git rev-parse --revs-only --no-flags --default saved "$@")
> 		git diff-tree $flags $save:base $save:work
> 	}
>
> It's a dense (and ancient style) plumbing code so needs a bit of
> explanation:
>
>  - The first git-rev-parse looks at "$@", discards everything
>    that are not options and discards object names.  So 'git save
>    show -p some' will give you "-p" in flags.
>
>  - The second one discards flags, and grabs 'some' out of '-p
>    some', or defaults to "saved".