[PATCH] 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 <redacted>
---
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

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

Johannes Schindelin wrote:

Hi,

On Wed, 4 Jul 2007, Johannes Sixt wrote:

quoted

-     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

Hi,

On Wed, 4 Jul 2007, Johannes Sixt wrote:

Johannes Schindelin wrote:

quoted

On Wed, 4 Jul 2007, Johannes Sixt wrote:

quoted

-     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

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 <redacted>
---
 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

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 <redacted>

Thanked-for-and-Acked-by: Johannes Schindelin [off-list ref]

Ciao,
Dscho