All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] bisect: remove unnecessary redirection
@ 2014-08-30 19:30 David Aguilar
  2014-08-30 19:30 ` [PATCH 2/2] stash: prefer --quiet over shell redirection David Aguilar
  2014-08-30 20:57 ` [PATCH 1/2] bisect: remove unnecessary redirection Johannes Sixt
  0 siblings, 2 replies; 5+ messages in thread
From: David Aguilar @ 2014-08-30 19:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Christian Couder, Ævar Arnfjörð Bjarmason,
	Jon Seymour

`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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] stash: prefer --quiet over shell redirection
  2014-08-30 19:30 [PATCH 1/2] bisect: remove unnecessary redirection David Aguilar
@ 2014-08-30 19:30 ` David Aguilar
  2014-08-30 21:07   ` Johannes Sixt
  2014-08-30 20:57 ` [PATCH 1/2] bisect: remove unnecessary redirection Johannes Sixt
  1 sibling, 1 reply; 5+ messages in thread
From: David Aguilar @ 2014-08-30 19:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Christian Couder, Ævar Arnfjörð Bjarmason,
	Jon Seymour

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] bisect: remove unnecessary redirection
  2014-08-30 19:30 [PATCH 1/2] bisect: remove unnecessary redirection David Aguilar
  2014-08-30 19:30 ` [PATCH 2/2] stash: prefer --quiet over shell redirection David Aguilar
@ 2014-08-30 20:57 ` Johannes Sixt
  1 sibling, 0 replies; 5+ messages in thread
From: Johannes Sixt @ 2014-08-30 20:57 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, git, Christian Couder,
	Ævar Arnfjörð Bjarmason, Jon Seymour

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] stash: prefer --quiet over shell redirection
  2014-08-30 19:30 ` [PATCH 2/2] stash: prefer --quiet over shell redirection David Aguilar
@ 2014-08-30 21:07   ` Johannes Sixt
  2014-08-31  7:35     ` David Aguilar
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Sixt @ 2014-08-30 21:07 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, git, Christian Couder,
	Ævar Arnfjörð Bjarmason, Jon Seymour

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] stash: prefer --quiet over shell redirection
  2014-08-30 21:07   ` Johannes Sixt
@ 2014-08-31  7:35     ` David Aguilar
  0 siblings, 0 replies; 5+ messages in thread
From: David Aguilar @ 2014-08-31  7:35 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Junio C Hamano, git, Christian Couder,
	Ævar Arnfjörð Bjarmason, Jon Seymour

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-08-31  7:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-30 19:30 [PATCH 1/2] bisect: remove unnecessary redirection David Aguilar
2014-08-30 19:30 ` [PATCH 2/2] stash: prefer --quiet over shell redirection David Aguilar
2014-08-30 21:07   ` Johannes Sixt
2014-08-31  7:35     ` David Aguilar
2014-08-30 20:57 ` [PATCH 1/2] bisect: remove unnecessary redirection Johannes Sixt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.