On Fri, Nov 2, 2012 at 5:03 PM, Stefano Lattarini
<stefano.lattarini@xxxxxxxxx> wrote:
> On 11/02/2012 04:42 PM, Felipe Contreras wrote:
>> What happens when you call this with:
>>
>> ./script "alias with spaces"
>>
> '$alias' will correctly expand to "alias with spaces". Try out:
>
> $ sh -c 'alias=$1; echo "$alias"' dummy '1 2*3'
> 1 2*3
>
> This works consistently with every known shell (even non-POSIX
> relics like Solaris /bin/sh).
All right.
>> _If_ we want this as POSIX, yeah.
>>
> Why don't we? Why add an extra requirement for a test that
>
> 1. can be easily written in POSIX shell, and
> 2. tests a feature that doesn't require bash to work (unless
> I'm sorely mistaken, that is)?
>
> Honest question. But of course, if the Git active contributors
> deem the extra requirement (which is not an invasive one, given
> how often bash is installed even on non-Linux systems) acceptable
> in order to have the test case simpler and clearer, feel free to
> disregard all my observations in this thread.
Because the code will be more complicated. Most of the changes
required are relatively small, so it's not a big issue, but I would
like to see how you replace the code that uses associative arrays. I
don't know know of a clean, simple way to do it in POSIX. If you can
find one, I don't see any strong reason to use bash.
>>>> +while read line; do
>>>> + case "$line" in
>>>>
>>> Useless double quoting (my previous observation about Git coding
>>> guidelines applies here as well, of course).
>>
>> What if line has multiple spaces?
>>
> Still no problem, as in the case of the 'alias=$1' assignment before:
>
> $ sh -c 'case $1 in *x" "x*) echo ok;; *) exit 1;; esac' dummy 'x x'
> ok
All right.
>>>> + echo "feature import-marks=$gitmarks"
>>>> + echo "feature export-marks=$gitmarks"
>>>> + git fast-export --use-done-feature --{import,export}-marks="$testgitmarks" $refs | \
>>>>
>>> Better avoid the tricky {foo,bar} bashism:
>>>
>>> git fast-export --use-done-feature \
>>> --import-marks="$testgitmarks" \
>>> --export-marks="$testgitmarks" \
>>> $refs | \
>>
>> If that's what we want, yeah.
>>
> Honestly, I find my longer-and-more-explicit version clearer, even
> if you can assume bash for your script. But that's a matter of
> personal preference (sorry for not stating that right away), so
> feel free to ignore it if you decide to keep the bash requirement
> in the end.
And I prefer the other form. I fact, I would prefer if both tools
simply had a --marks option set both, and didn't require the file to
be created beforehand. I've _never_ seen a situation where separate
marks for import and export made sense. But that's off-topic.
If you find a way to replace the associative arrays code, I have no
trouble in switching this to the POSIX version.
Cheers.
--
Felipe Contreras
--
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