[Parted-edge] Patches waiting for bindings.git

[Jim Meyering]

Otavio Salvador <otavio at debian.org> wrote:
> Jim Meyering <jim at meyering.net> writes:
>
>> Otavio Salvador <otavio at debian.org> wrote:
>>> I've preparated two patches for bindings.git tree and would like to
>>> get feedback before commiting them.
>>
>> +if [ ! -x /usr/bin/astyle ]; then
>> +    echo "ERROR: You need to have astyle installed."
>> +    exit 1
>> +fi
>>
>> Don't test for it that way.
>> What if it's installed in /usr/local/bin or $HOME/bin?
>> You don't want it to fail in those cases.
>> Instead, do this:
>>
>> ( astyle --version > /dev/null 2>&1 ) || {
>>   echo "$0: ERROR: You need to have astyle installed."
>>   exit 1
>> }
>>
>> Or just omit the test altogether.
>> When the user sees "astyle: command not found", they usually
>> understand what they need to do.
>
> I've change it to which since --version always fail :(

I reiterate the suggestion to simply remove it :)
If you insist on using such a check, don't use 'which',
since it's not always available, and when it is,
sometimes it's a separate command.

First, astyle --version doing "exit(1)" is a bug: it's
contrary to the GNU Coding Standards.
To work around it, you could instead do this:

( astyle --version | grep 'Artistic Style' > /dev/null 2>&1 ) || {
  echo "$0: ERROR: You need to have astyle installed."
  exit 1
}

Regarding this,

  +astyle --mode=c \
  +       --indent=spaces=4 \
  +       --indent-classes \
  +       --indent-switches \
  +       --indent-cases \
  +       --indent-namespaces \
  +       --brackets=linux \
  +       -p \
  +       --convert-tabs \
  +       --break-blocks \
  +       --break-blocks=all \
  +       --pad=oper \
  +       --pad=paren-out \
  +       --unpad=paren \
  +       $*

you should use "$@" (with the double quotes), not $*.
And if you start with

  exec astyle ...

you save a process and automatically make the script
exit propagate astyle's exit code "out".