Instead of using eval which causes problems when a URL contains an appropriately escaped ampersand (\&). Cc: peff@xxxxxxxx Cc: chriscool@xxxxxxxxxxxxx Cc: jepler@xxxxxxxxxxxxxx Signed-off-by: Chris Packham <judge.packham@xxxxxxxxx> --- > Which implies that "$browser_path" must be the actual > executable. In which case, I would think that: > > "$browser_path" "$@" & > > would be the right thing. And indeed, that is what the firefox arm of > the case statement does. But chrome, konqueror, and others use eval. So here is my attempt at a fix for kfmclient. For what it's worth I've included a testcase that detects my problem. I'm not sure if the testcase is really worth it because the test library suppresses X applications and even if it didn't the testcase is fairly trivial and might just annoy people by opening web-browsers (and it snaps up the last t99xx prefix). git-web--browse.sh | 4 ++-- t/t9901-git-web--browse.sh | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100755 t/t9901-git-web--browse.sh diff --git a/git-web--browse.sh b/git-web--browse.sh index e9de241..1164a22 100755 --- a/git-web--browse.sh +++ b/git-web--browse.sh @@ -164,10 +164,10 @@ konqueror) # It's simpler to use kfmclient to open a new tab in konqueror. browser_path="$(echo "$browser_path" | sed -e 's/konqueror$/kfmclient/')" type "$browser_path" > /dev/null 2>&1 || die "No '$browser_path' found." - eval "$browser_path" newTab "$@" + "$browser_path" newTab "$@" & ;; kfmclient) - eval "$browser_path" newTab "$@" + "$browser_path" newTab "$@" & ;; *) "$browser_path" "$@" & diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh new file mode 100755 index 0000000..7ed38a0 --- /dev/null +++ b/t/t9901-git-web--browse.sh @@ -0,0 +1,43 @@ +#!/bin/sh +# +# Copyright (c) 2011 Chris Packham +# + +test_description='git web--browse basic tests + +This test checks that git web--browse can handle various valid URLs with +the supported browsers that are installed on the host system.' + +. ./test-lib.sh + +test -x /usr/bin/firefox && test_set_prereq FIREFOX +test -x /usr/bin/konqueror && test_set_prereq KONQUEROR +test -x /usr/bin/google-chrome && test_set_prereq CHROME +test -x /usr/bin/opera && test_set_prereq OPERA + +test_expect_success \ + 'accepts a URL with an ampersand in it (default)' ' + git web--browse http://example.com/foo\&bar/ +' + +test_expect_success FIREFOX \ + 'accepts a URL with an ampersand in it (firefox)' ' + git web--browse --browser=firefox http://example.com/foo\&bar/ +' + +test_expect_success KONQUEROR \ + 'accepts a URL with an ampersand in it (konqueror)' ' + git web--browse --browser=konqueror http://example.com/foo\&bar/ +' + +test_expect_success OPERA \ + 'accepts a URL with an ampersand in it (opera)' ' + git web--browse --browser=opera http://example.com/foo\&bar/ +' + +test_expect_success CHROME \ + 'accepts a URL with an ampersand in it (chrome)' ' + git web--browse --browser=google-chrome http://example.com/foo\&bar/ +' + +test_done -- 1.7.7.rc1.3.g5593.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html