Re: [PATCH (3rd try)] Add git-stash script

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1104 bytes --]

Hi,

On Sat, 30 Jun 2007, しらいしななこ wrote:

> diff --git a/git-stash.sh b/git-stash.sh
> [...]
> +	printf >&2 'Saved WIP on %s\n' "$msg"

You have an awful lot of printfs in the code. Why not just use echos?

> +list_stash () {
> +	git-log --pretty=oneline -g "$@" $ref_stash |

Wouldn't you want "--default $ref_stash" here?

> +apply_stash () {
> +	git-diff-files --quiet ||
> +		die 'Cannot restore on top of a dirty state'

You meant "no_changes", right? I think you miss changes in the index 
otherwise.

> +		git-diff --cached --name-only --diff-filter=A $c_tree >"$a" &&
> +		git-read-tree --reset $c_tree &&
> +		git-update-index --add --stdin <"$a" ||
> +			die "Cannot unstage modified files"

Isn't there a way to avoid the temporary file here?

> +	else
> +		# Merge conflict
> +		exit 1

Since $? is already != 0, and it might tell the savvy user what kind of 
error merge-recursive returned, why not use "exit", which is equivalent to 
"exit $?"?

> +		set x -n 10
> +		shift

This is more elegantly written as "set -- -n 10", or in our context even 
"set -- -10".

Ciao,
Dscho