threads / patch / 8851

patchfilter-branch: Avoid an error message in the map function.

Subject: [PATCH] filter-branch: Avoid an error message in the map function.

## tl;dr

6 messages between Jul 4, 2007 and Jul 4, 2007. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Johannes Sixt· Jul 4, 2007, 07:54 UTC · lore

When the map function didn't find the rewritten commit of the passed in original id, it printed the original id, but it still fell through to the 'cat', which failed with an error message.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Is the sequence of && and || ok, or do you prefer if-then-else-fi?

-- Hannes

 git-filter-branch.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Show changes to git-filter-branch.sh +3 −2

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 22fb5bf..ff1cbcf 100644
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -16,8 +16,9 @@ USAGE="git-filter-branch [-d TEMPDIR] [FILTERS] DESTBRANCH [REV-RANGE]"
 map()
 {
 	# if it was not rewritten, take the original
-	test -r "$workdir/../map/$1" || echo "$1"
-	cat "$workdir/../map/$1"
+	test -r "$workdir/../map/$1" &&
+	cat "$workdir/../map/$1" ||
+	echo "$1"
 }
 
 # When piped a commit, output a script to set the ident of either

-- 
1.5.3.rc0.5.g7cd9

Johannes Schindelin· Jul 4, 2007, 11:08 UTC · re: Johannes Sixt · lore

Re: [PATCH] filter-branch: Avoid an error message in the map function.

Hi,

On Wed, 4 Jul 2007, Johannes Sixt wrote:

Show 5 quoted lines

> -	test -r "$workdir/../map/$1" || echo "$1"
> -	cat "$workdir/../map/$1"
> +	test -r "$workdir/../map/$1" &&
> +	cat "$workdir/../map/$1" ||
> +	echo "$1"

I think this does not do what you want. If I read it correctly, it will not do anything if $workdir/../map/$1 is not readable. I think you need this:

	(test -r "$workdir/../map/$1" &&
	cat "$workdir/../map/$1") ||
	echo "$1"

But that is a little too cute, so I personally would prefer an if-then-else-fi, because that is the idea of that code snippet.

Ciao, Dscho

Johannes Sixt· Jul 4, 2007, 11:45 UTC · re: Johannes Schindelin · lore

Re: [PATCH] filter-branch: Avoid an error message in the map function.

Johannes Schindelin wrote:

Show 21 quoted lines

> 
> Hi,
> 
> On Wed, 4 Jul 2007, Johannes Sixt wrote:
> 
> > -     test -r "$workdir/../map/$1" || echo "$1"
> > -     cat "$workdir/../map/$1"
> > +     test -r "$workdir/../map/$1" &&
> > +     cat "$workdir/../map/$1" ||
> > +     echo "$1"
> 
> I think this does not do what you want. If I read it correctly, it will
> not do anything if $workdir/../map/$1 is not readable. I think you need
> this:
> 
>         (test -r "$workdir/../map/$1" &&
>         cat "$workdir/../map/$1") ||
>         echo "$1"
> 
> But that is a little too cute, so I personally would prefer an
> if-then-else-fi, because that is the idea of that code snippet.

It does do what I think it should do. I tested it. Your elaborate version is not required. The reason is that in the shell && and || have equal precedence; if there is ... && cmd ... then cmd is run if the most recent result is success, and if there is ... || cmd ... then cmd is run if the most recent result is failure; in both cases cmd is otherwise skipped and does not count as "most recent result".

-- Hannes

Johannes Schindelin· Jul 4, 2007, 11:49 UTC · re: Johannes Sixt · lore

Re: [PATCH] filter-branch: Avoid an error message in the map function.

Hi,

On Wed, 4 Jul 2007, Johannes Sixt wrote:

Show 22 quoted lines

> Johannes Schindelin wrote:
> > 
> > On Wed, 4 Jul 2007, Johannes Sixt wrote:
> > 
> > > -     test -r "$workdir/../map/$1" || echo "$1"
> > > -     cat "$workdir/../map/$1"
> > > +     test -r "$workdir/../map/$1" &&
> > > +     cat "$workdir/../map/$1" ||
> > > +     echo "$1"
> > 
> > I think this does not do what you want. If I read it correctly, it will
> > not do anything if $workdir/../map/$1 is not readable. I think you need
> > this:
> > 
> >         (test -r "$workdir/../map/$1" &&
> >         cat "$workdir/../map/$1") ||
> >         echo "$1"
> > 
> > But that is a little too cute, so I personally would prefer an
> > if-then-else-fi, because that is the idea of that code snippet.
> 
> It does do what I think it should do. I tested it.

Okay. But take me as an example of an average programmer. I got confused. Therefore I would greatly appreciate it, if it were written with if-then-else-fi, because I get less confused then.

Thanks, Dscho

Johannes Sixt· Jul 4, 2007, 12:08 UTC · re: Johannes Schindelin · lore

[PATCH take 2] filter-branch: Avoid an error message in the map function.

When the map function didn't find the rewritten commit of the passed in original id, it printed the original id, but it still fell through to the 'cat', which failed with an error message.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 git-filter-branch.sh |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

Show changes to git-filter-branch.sh +6 −2

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 22fb5bf..5fa9b61 100644
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -16,8 +16,12 @@
 map()
 {
 	# if it was not rewritten, take the original
-	test -r "$workdir/../map/$1" || echo "$1"
-	cat "$workdir/../map/$1"
+	if test -r "$workdir/../map/$1"
+	then
+		cat "$workdir/../map/$1"
+	else
+		echo "$1"
+	fi
 }
 
 # When piped a commit, output a script to set the ident of either

-- 
1.5.3.rc0.5.g7cd9

Johannes Schindelin· Jul 4, 2007, 12:51 UTC · re: Johannes Sixt · lore

Re: [PATCH take 2] filter-branch: Avoid an error message in the map function.

Hi,

On Wed, 4 Jul 2007, Johannes Sixt wrote:

Show 5 quoted lines

> When the map function didn't find the rewritten commit of the passed in
> original id, it printed the original id, but it still fell through to
> the 'cat', which failed with an error message.
> 
> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>

Thanked-for-and-Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Ciao, Dscho