Re: [PATCH] Fixing path quoting issues
- From
Johannes Sixt <j.sixt@viscovery.net>
- Date
- Oct 11, 2007, 06:19 UTC
- Message-ID
- <470DC05A.8020209@viscovery.net>
- In-Reply-To
- <11920508172434-git-send-email-jon.delStrother@bestbefore.tv>
Jonathan del Strother schrieb:
> + cmt=`cat "$dotest/current"`
This is ok, but...
> + prev_head="`cat \"$dotest/prev_head\"`"
... there are shells out there in the wild that will get badly confused by this sort of quoting and escaping. Butter use
prev_head=$(cat "$dotest/prev_head")
> -VISUAL="$(pwd)/fake-editor.sh" > +VISUAL="'$(pwd)/fake-editor.sh'"
Huh? This looks very wrong. What are the extra quotes needed for? If they are really needed, isn't this a bug in git-rebase--interactive.sh?
> - git-commit -F msg -m amending ." > + git-commit -F msg -m amending ."
You fix whitespace...
Show 5 quoted lines
> test_expect_success \ > - "using message from other commit" \ > - "git-commit -C HEAD^ ." > + "using message from other commit" \ > + "git-commit -C HEAD^ ."
... and you break it. More of these follow. Don't do that, it makes patch review unnecessarily hard.
I question the usefulness of this patch. Why only fix breakage due to spaces in the path? What about single-quotes, double-quotes? IMHO, it's not too much of a burden for developers to require "sane" build directory paths.
-- Hannes