Re: [PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows

On Fri, Nov 11, 2011 at 08:18:03PM +0400, Alexey Shumkin wrote:

Firefox on Windows by default is placed in "C:\Program Files\Mozilla Firefox"
folder, i.e. its path contains spaces. Before running this browser "git-web--browse"
tests version of Firefox to decide whether to use "-new-tab" option or not.

Quote browser path to avoid error during this test.

Thanks. I even noticed this bug early on in the previous discussion:

  http://article.gmane.org/gmane.comp.version-control.git/181600

but forgot about it by the time the final patch rolled around. Your fix
looks correct, but:

 test_expect_success \
+	'Firefox below v2.0 paths are properly quoted' '
+	echo fake: http://example.com/foo >expect &&
+	cat >"fake browser" <<-\EOF &&
+	#!/bin/sh
+
+	if [ "$1" == "-version" ]; then

Using "==" is a bashism. Just use "=".

Also, a style nit, but we usually spell this "test" and not "[". I admit
I don't care much, though.

+		# Firefox (in contrast to w3m) is run in background (with &)
+		# so redirect output to "actual"
+		echo fake: "$@" > actual
+	fi
+	EOF
+	chmod +x "fake browser" &&
+	git config browser.firefox.path "`pwd`/fake browser" &&
+	git web--browse --browser=firefox \
+		http://example.com/foo &&
+	test_cmp expect actual

Hmm. So we are running the fake browser in the background, but then
check that it has written something as soon as web--browse exits. Isn't
that a race condition? I.e., we could run "test_cmp" before the browser
has actually written anything?

I'm not sure there's a good way to do it.  You would need either to wait
some pre-determined "it could not possibly take it longer than N seconds
to run" sleep, or we need some kind of synchronization point. We can't
wait call "wait" on the child PID (if we even have it, because it's not
our child).

-Peff