* [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 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
* 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
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).