# [PATCH 1/2] bisect: remove unnecessary redirection

5 messages from 2014-08-30 to 2014-08-31. Participants: David Aguilar, Johannes Sixt.
Thread: https://gitlist.dev/t/37457

## David Aguilar, 2014-08-30 19:30

Subject: [PATCH 1/2] bisect: remove unnecessary redirection
Message-ID: <1409427029-65886-1-git-send-email-davvid@gmail.com>
URL: https://gitlist.dev/e/1409427029-65886-1-git-send-email-davvid%40gmail.com

```
`git rev-parse` is being called with --quiet so there's no need to
redirect to /dev/null.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-bisect.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 1e0d602..c1c2321 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -365,7 +365,7 @@ bisect_reset() {
 	}
 	case "$#" in
 	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
-	1) git rev-parse --quiet --verify "$1^{commit}" >/dev/null || {
+	1) git rev-parse --quiet --verify "$1^{commit}" || {
 			invalid="$1"
 			die "$(eval_gettext "'\$invalid' is not a valid commit")"
 		}
-- 
2.1.0.29.g9b751c4

```

## David Aguilar, 2014-08-30 19:30

Subject: [PATCH 2/2] stash: prefer --quiet over shell redirection
Message-ID: <1409427029-65886-2-git-send-email-davvid@gmail.com>
URL: https://gitlist.dev/e/1409427029-65886-2-git-send-email-davvid%40gmail.com
In-Reply-To: <1409427029-65886-1-git-send-email-davvid@gmail.com>

```
Use `git rev-parse --quiet` instead of redirecting to /dev/null.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-stash.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index bcc757b..5a5185b 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -50,7 +50,7 @@ clear_stash () {
 	then
 		die "$(gettext "git stash clear with parameters is unimplemented")"
 	fi
-	if current=$(git rev-parse --verify $ref_stash 2>/dev/null)
+	if current=$(git rev-parse --quiet --verify $ref_stash)
 	then
 		git update-ref -d $ref_stash $current
 	fi
@@ -67,7 +67,7 @@ create_stash () {
 	fi
 
 	# state of the base commit
-	if b_commit=$(git rev-parse --verify HEAD)
+	if b_commit=$(git rev-parse --quiet --verify HEAD)
 	then
 		head=$(git rev-list --oneline -n 1 HEAD --)
 	else
@@ -292,7 +292,7 @@ save_stash () {
 }
 
 have_stash () {
-	git rev-parse --verify $ref_stash >/dev/null 2>&1
+	git rev-parse --quiet --verify $ref_stash
 }
 
 list_stash () {
@@ -392,12 +392,12 @@ parse_flags_and_rev()
 		;;
 	esac
 
-	REV=$(git rev-parse --quiet --symbolic --verify "$1" 2>/dev/null) || {
+	REV=$(git rev-parse --quiet --symbolic --verify "$1") || {
 		reference="$1"
 		die "$(eval_gettext "\$reference is not valid reference")"
 	}
 
-	i_commit=$(git rev-parse --quiet --verify "$REV^2" 2>/dev/null) &&
+	i_commit=$(git rev-parse --quiet --verify "$REV^2") &&
 	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&
 	s=$1 &&
 	w_commit=$1 &&
@@ -409,7 +409,7 @@ parse_flags_and_rev()
 	test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" &&
 	IS_STASH_REF=t
 
-	u_commit=$(git rev-parse --quiet --verify "$REV^3" 2>/dev/null) &&
+	u_commit=$(git rev-parse --quiet --verify "$REV^3") &&
 	u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)
 }
 
@@ -531,7 +531,7 @@ drop_stash () {
 		die "$(eval_gettext "\${REV}: Could not drop stash entry")"
 
 	# clear_stash if we just dropped the last stash entry
-	git rev-parse --verify "$ref_stash@{0}" >/dev/null 2>&1 || clear_stash
+	git rev-parse --quiet --verify "$ref_stash@{0}" || clear_stash
 }
 
 apply_to_branch () {
-- 
2.1.0.29.g9b751c4

```

## Johannes Sixt, 2014-08-30 20:57

Subject: Re: [PATCH 1/2] bisect: remove unnecessary redirection
Message-ID: <54023AA1.40709@kdbg.org>
URL: https://gitlist.dev/e/54023AA1.40709%40kdbg.org
In-Reply-To: <1409427029-65886-1-git-send-email-davvid@gmail.com>

```
Am 30.08.2014 21:30, schrieb David Aguilar:
> `git rev-parse` is being called with --quiet so there's no need to
> redirect to /dev/null.
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>  git-bisect.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-bisect.sh b/git-bisect.sh
> index 1e0d602..c1c2321 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -365,7 +365,7 @@ bisect_reset() {
>  	}
>  	case "$#" in
>  	0) branch=$(cat "$GIT_DIR/BISECT_START") ;;
> -	1) git rev-parse --quiet --verify "$1^{commit}" >/dev/null || {
> +	1) git rev-parse --quiet --verify "$1^{commit}" || {

This is wrong. The redirection quells stdout, but --quiet is about stderr.

>  			invalid="$1"
>  			die "$(eval_gettext "'\$invalid' is not a valid commit")"
>  		}
> 

-- Hannes

```

## Johannes Sixt, 2014-08-30 21:07

Subject: Re: [PATCH 2/2] stash: prefer --quiet over shell redirection
Message-ID: <54023D01.6090504@kdbg.org>
URL: https://gitlist.dev/e/54023D01.6090504%40kdbg.org
In-Reply-To: <1409427029-65886-2-git-send-email-davvid@gmail.com>

```
Am 30.08.2014 21:30, schrieb David Aguilar:
> Use `git rev-parse --quiet` instead of redirecting to /dev/null.
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>  git-stash.sh | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index bcc757b..5a5185b 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -50,7 +50,7 @@ clear_stash () {
>  	then
>  		die "$(gettext "git stash clear with parameters is unimplemented")"
>  	fi
> -	if current=$(git rev-parse --verify $ref_stash 2>/dev/null)
> +	if current=$(git rev-parse --quiet --verify $ref_stash)
>  	then
>  		git update-ref -d $ref_stash $current
>  	fi
> @@ -67,7 +67,7 @@ create_stash () {
>  	fi
>  
>  	# state of the base commit
> -	if b_commit=$(git rev-parse --verify HEAD)
> +	if b_commit=$(git rev-parse --quiet --verify HEAD)
>  	then
>  		head=$(git rev-list --oneline -n 1 HEAD --)
>  	else

The else branch calls die; wouldn't it be useful to keep the error
message of rev-parse?

> @@ -292,7 +292,7 @@ save_stash () {
>  }
>  
>  have_stash () {
> -	git rev-parse --verify $ref_stash >/dev/null 2>&1
> +	git rev-parse --quiet --verify $ref_stash

This change is not correct: --quiet removes only output on stderr, but
not that on stdout.

>  }
>  
>  list_stash () {
> @@ -392,12 +392,12 @@ parse_flags_and_rev()
>  		;;
>  	esac
>  
> -	REV=$(git rev-parse --quiet --symbolic --verify "$1" 2>/dev/null) || {
> +	REV=$(git rev-parse --quiet --symbolic --verify "$1") || {
>  		reference="$1"
>  		die "$(eval_gettext "\$reference is not valid reference")"
>  	}
>  
> -	i_commit=$(git rev-parse --quiet --verify "$REV^2" 2>/dev/null) &&
> +	i_commit=$(git rev-parse --quiet --verify "$REV^2") &&
>  	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&

I see another rev-parse that you did not modify. An omission?

>  	s=$1 &&
>  	w_commit=$1 &&
> @@ -409,7 +409,7 @@ parse_flags_and_rev()
>  	test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" &&
>  	IS_STASH_REF=t
>  
> -	u_commit=$(git rev-parse --quiet --verify "$REV^3" 2>/dev/null) &&
> +	u_commit=$(git rev-parse --quiet --verify "$REV^3") &&
>  	u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)

I see yet another rev-parse that you did not modify.

>  }
>  
> @@ -531,7 +531,7 @@ drop_stash () {
>  		die "$(eval_gettext "\${REV}: Could not drop stash entry")"
>  
>  	# clear_stash if we just dropped the last stash entry
> -	git rev-parse --verify "$ref_stash@{0}" >/dev/null 2>&1 || clear_stash
> +	git rev-parse --quiet --verify "$ref_stash@{0}" || clear_stash

Again not correct.

>  }
>  
>  apply_to_branch () {
> 

-- Hannes

```

## David Aguilar, 2014-08-31 07:35

Subject: Re: [PATCH 2/2] stash: prefer --quiet over shell redirection
Message-ID: <20140831073550.GA66236@gmail.com>
URL: https://gitlist.dev/e/20140831073550.GA66236%40gmail.com
In-Reply-To: <54023D01.6090504@kdbg.org>

```
On Sat, Aug 30, 2014 at 11:07:13PM +0200, Johannes Sixt wrote:
> Am 30.08.2014 21:30, schrieb David Aguilar:
> > @@ -392,12 +392,12 @@ parse_flags_and_rev()
> >  		;;
> >  	esac
> >  
> > -	REV=$(git rev-parse --quiet --symbolic --verify "$1" 2>/dev/null) || {
> > +	REV=$(git rev-parse --quiet --symbolic --verify "$1") || {
> >  		reference="$1"
> >  		die "$(eval_gettext "\$reference is not valid reference")"
> >  	}
> >  
> > -	i_commit=$(git rev-parse --quiet --verify "$REV^2" 2>/dev/null) &&
> > +	i_commit=$(git rev-parse --quiet --verify "$REV^2") &&
> >  	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&
> 
> I see another rev-parse that you did not modify. An omission?

The docs for --quiet say, "Only meaningful in --verify mode", so I didn't touch
the non-verify call-sites.

Thanks for the review. I'll address your notes and send a v2 shortly.
-- 
David

```