{"thread":{"id":"8851","subject":"[PATCH] filter-branch: Avoid an error message in the map function.","startedAt":"2007-07-04T07:54:07Z","lastAt":"2007-07-04T12:51:43Z","messageCount":6,"participants":["Johannes Sixt","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"46449","messageId":"11835356473029-git-send-email-johannes.sixt@telecom.at","threadId":"8851","inReplyTo":null,"subject":"[PATCH] filter-branch: Avoid an error message in the map function.","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2007-07-04T07:54:07Z","receivedAt":"2007-07-04T07:54:07Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"When the map function didn't find the rewritten commit of the passed in\noriginal id, it printed the original id, but it still fell through to\nthe 'cat', which failed with an error message.\n\nSigned-off-by: Johannes Sixt \n---\nIs the sequence of && and || ok, or do you prefer if-then-else-fi?\n\n-- Hannes\n\n git-filter-branch.sh | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 22fb5bf..ff1cbcf 100644\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -16,8 +16,9 @@ USAGE=\"git-filter-branch [-d TEMPDIR] [FILTERS] DESTBRANCH [REV-RANGE]\"\n map()\n {\n \t# if it was not rewritten, take the original\n-\ttest -r \"$workdir/../map/$1\" || echo \"$1\"\n-\tcat \"$workdir/../map/$1\"\n+\ttest -r \"$workdir/../map/$1\" &&\n+\tcat \"$workdir/../map/$1\" ||\n+\techo \"$1\"\n }\n \n # When piped a commit, output a script to set the ident of either\n-- \n1.5.3.rc0.5.g7cd9\n"},{"id":"46465","messageId":"Pine.LNX.4.64.0707041205250.4071@racer.site","threadId":"8851","inReplyTo":"11835356473029-git-send-email-johannes.sixt@telecom.at","subject":"Re: [PATCH] filter-branch: Avoid an error message in the map function.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-04T11:08:26Z","receivedAt":"2007-07-04T11:08:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Jul 2007, Johannes Sixt wrote:\n\n> -\ttest -r \"$workdir/../map/$1\" || echo \"$1\"\n> -\tcat \"$workdir/../map/$1\"\n> +\ttest -r \"$workdir/../map/$1\" &&\n> +\tcat \"$workdir/../map/$1\" ||\n> +\techo \"$1\"\n\nI think this does not do what you want. If I read it correctly, it will \nnot do anything if $workdir/../map/$1 is not readable. I think you need \nthis:\n\n\t(test -r \"$workdir/../map/$1\" &&\n\tcat \"$workdir/../map/$1\") ||\n\techo \"$1\"\n\nBut that is a little too cute, so I personally would prefer an \nif-then-else-fi, because that is the idea of that code snippet.\n\nCiao,\nDscho\n"},{"id":"46469","messageId":"468B8852.CB36968@eudaptics.com","threadId":"8851","inReplyTo":"Pine.LNX.4.64.0707041205250.4071@racer.site","subject":"Re: [PATCH] filter-branch: Avoid an error message in the map function.","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-07-04T11:45:22Z","receivedAt":"2007-07-04T11:45:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin wrote:\n> \n> Hi,\n> \n> On Wed, 4 Jul 2007, Johannes Sixt wrote:\n> \n> > - test -r \"$workdir/../map/$1\" || echo \"$1\"\n> > - cat \"$workdir/../map/$1\"\n> > + test -r \"$workdir/../map/$1\" &&\n> > + cat \"$workdir/../map/$1\" ||\n> > + echo \"$1\"\n> \n> I think this does not do what you want. If I read it correctly, it will\n> not do anything if $workdir/../map/$1 is not readable. I think you need\n> this:\n> \n> (test -r \"$workdir/../map/$1\" &&\n> cat \"$workdir/../map/$1\") ||\n> echo \"$1\"\n> \n> But that is a little too cute, so I personally would prefer an\n> if-then-else-fi, because that is the idea of that code snippet.\n\nIt does do what I think it should do. I tested it. Your elaborate\nversion is not required. The reason is that in the shell && and || have\nequal precedence; if there is ... && cmd ... then cmd is run if the most\nrecent result is success, and if there is ... || cmd ... then cmd is run\nif the most recent result is failure; in both cases cmd is otherwise\nskipped and does not count as \"most recent result\".\n\n-- Hannes\n"},{"id":"46470","messageId":"Pine.LNX.4.64.0707041247520.4071@racer.site","threadId":"8851","inReplyTo":"468B8852.CB36968@eudaptics.com","subject":"Re: [PATCH] filter-branch: Avoid an error message in the map function.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-04T11:49:03Z","receivedAt":"2007-07-04T11:49:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Jul 2007, Johannes Sixt wrote:\n\n> Johannes Schindelin wrote:\n> > \n> > On Wed, 4 Jul 2007, Johannes Sixt wrote:\n> > \n> > > - test -r \"$workdir/../map/$1\" || echo \"$1\"\n> > > - cat \"$workdir/../map/$1\"\n> > > + test -r \"$workdir/../map/$1\" &&\n> > > + cat \"$workdir/../map/$1\" ||\n> > > + echo \"$1\"\n> > \n> > I think this does not do what you want. If I read it correctly, it will\n> > not do anything if $workdir/../map/$1 is not readable. I think you need\n> > this:\n> > \n> > (test -r \"$workdir/../map/$1\" &&\n> > cat \"$workdir/../map/$1\") ||\n> > echo \"$1\"\n> > \n> > But that is a little too cute, so I personally would prefer an\n> > if-then-else-fi, because that is the idea of that code snippet.\n> \n> It does do what I think it should do. I tested it.\n\nOkay. But take me as an example of an average programmer. I got confused. \nTherefore I would greatly appreciate it, if it were written with \nif-then-else-fi, because I get less confused then.\n\nThanks,\nDscho\n"},{"id":"46473","messageId":"468B8DB1.9A4CF1E6@eudaptics.com","threadId":"8851","inReplyTo":"Pine.LNX.4.64.0707041247520.4071@racer.site","subject":"[PATCH take 2] filter-branch: Avoid an error message in the map function.","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-07-04T12:08:17Z","receivedAt":"2007-07-04T12:08:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"When the map function didn't find the rewritten commit of the passed in\noriginal id, it printed the original id, but it still fell through to\nthe 'cat', which failed with an error message.\n\nSigned-off-by: Johannes Sixt \n---\n git-filter-branch.sh | 8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 22fb5bf..5fa9b61 100644\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -16,8 +16,12 @@\n map()\n {\n \t# if it was not rewritten, take the original\n-\ttest -r \"$workdir/../map/$1\" || echo \"$1\"\n-\tcat \"$workdir/../map/$1\"\n+\tif test -r \"$workdir/../map/$1\"\n+\tthen\n+\t\tcat \"$workdir/../map/$1\"\n+\telse\n+\t\techo \"$1\"\n+\tfi\n }\n \n # When piped a commit, output a script to set the ident of either\n-- \n1.5.3.rc0.5.g7cd9\n"},{"id":"46477","messageId":"Pine.LNX.4.64.0707041351010.4071@racer.site","threadId":"8851","inReplyTo":"468B8DB1.9A4CF1E6@eudaptics.com","subject":"Re: [PATCH take 2] filter-branch: Avoid an error message in the map function.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-04T12:51:43Z","receivedAt":"2007-07-04T12:51:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Jul 2007, Johannes Sixt wrote:\n\n> When the map function didn't find the rewritten commit of the passed in\n> original id, it printed the original id, but it still fell through to\n> the 'cat', which failed with an error message.\n> \n> Signed-off-by: Johannes Sixt \n\nThanked-for-and-Acked-by: Johannes Schindelin \n\nCiao,\nDscho\n"}]}