On 2013-10-29 04:41, Felipe Contreras wrote:
> Richard Hansen wrote:
>> Signed-off-by: Richard Hansen <rhansen@xxxxxxx>
>> ---
>> git-remote-testgit.sh | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
>> index 6d2f282..80546c1 100755
>> --- a/git-remote-testgit.sh
>> +++ b/git-remote-testgit.sh
>> @@ -6,6 +6,7 @@ url=$2
>>
>> dir="$GIT_DIR/testgit/$alias"
>> prefix="refs/testgit/$alias"
>> +forcearg=
>>
>> default_refspec="refs/heads/*:${prefix}/heads/*"
>>
>> @@ -39,6 +40,7 @@ do
>> fi
>> test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
>> test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update"
>> + echo 'option'
>> echo
>> ;;
>> list)
>> @@ -93,6 +95,7 @@ do
>> before=$(git for-each-ref --format=' %(refname) %(objectname) ')
>>
>> git fast-import \
>> + ${forcearg} \
>> ${testgitmarks:+"--import-marks=$testgitmarks"} \
>> ${testgitmarks:+"--export-marks=$testgitmarks"} \
>> --quiet
>> @@ -115,6 +118,21 @@ do
>>
>> echo
>> ;;
>> + option\ *)
>> + read cmd opt val <<EOF
>> +${line}
>> +EOF
>
> We can do <<-EOF to align this properly.
Good point. I personally avoid tabs whenever possible, and <<- only
works with tabs, so I'm in the habit of doing <<EOF.
>
> Also, I don't see why all the variables are ${foo} instead of $foo.
I'm in the habit of doing ${foo} because I like the consistency --
sometimes you need them to disambiguate, and sometimes you need special
expansions like ${foo##bar} or ${foo:-bar}.
In this case it's actually less consistent to do ${foo} because the rest
of the file doesn't use {} when not needed, so I agree with your change.
>
>> + case ${opt} in
>> + force)
>
> I think the convention is to align these:
>
> case $opt in
> force)
The existing case statement in this file indents the patterns the same
amount as the case statement, so this should be aligned to match.
In general I rarely see the case patterns indented at the same level as
the case statement, possibly because Emacs shell-mode indents the
patterns more than the case statement (by default). The POSIX spec
contains a mix of styles:
* the normative text documenting the format of a 'case' construct
indents the patterns more than the 'case' statement
* two of the four non-normative examples indent the patterns
more than the 'case' statements; the other two do not
>
>> + case ${val} in
>> + true) forcearg=--force; echo 'ok';;
>> + false) forcearg=; echo 'ok';;
>> + *) printf %s\\n "error '${val}'\
>> + is not a valid value for option ${opt}";;
>
> I think this is packing a lot of stuff and it's not that readable.
>
> Moreover, this is not for production purposes, it's for testing purposes and a
> guideline, I think this suffices.
>
>
> option\ *)
> read cmd opt val <<-EOF
> $line
> EOF
> case $opt in
> force)
> test $val = "true" && force="true" || force=
> echo "ok"
> ;;
> *)
> echo "unsupported"
> ;;
> esac
> ;;
Works for me.
>
> But this is definetly good to have, will merge.
Thanks,
Richard
--
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