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

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

[Johannes Sixt]

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(-)

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

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

[Johannes Schindelin]

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.

Ciao,
Dscho

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

[Johannes Sixt]

Johannes Schindelin wrote:
> 
> 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

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

[Johannes Schindelin]

Hi,

On Wed, 4 Jul 2007, Johannes Sixt wrote:

> 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

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

[Johannes Sixt]

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(-)

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

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

[Johannes Schindelin]

Hi,

On Wed, 4 Jul 2007, Johannes Sixt wrote:

> 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