[Parted-edge] Patches waiting for bindings.git

Jim Meyering jim at meyering.net
Fri May 11 18:24:14 UTC 2007


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.

+astyle --mode=c \
+       --indent=spaces=4 \
+       --indent-classes \
+       --indent-switches \
+       --indent-cases \
+       --indent-namespaces \

Does --indent-namespaces make sense with --mode=c ?

+       --brackets=linux \
+       -p \
+       --convert-tabs \
+       --break-blocks \
+       --break-blocks=all \
+       $(find . -iname '*.h' -o -iname '*.c' -o -iname '*.hpp' -o \
+                -iname '*.cpp' -o -iname '*.cc')

That will fail in a hierarchy with too many (or too long)
names, or with names including embedded newlines.  Instead,
do this, assuming you don't mind depending on GNU find and GNU xargs:

  find .                 \
    '('                  \
       -iname '*.h' -o   \
       -iname '*.c' -o   \
       -iname '*.hpp' -o \
       -iname '*.cpp' -o \
       -iname '*.cc'     \
    ')'                  \
    -print0              \
  | xargs -0             \
      astyle             \
        --mode=c         \
        --indent=spaces=4   \
        --indent-classes    \
        --indent-switches   \
        --indent-cases      \
        --indent-namespaces \
        --brackets=linux    \
        -p                  \
        --convert-tabs      \
        --break-blocks      \
        --break-blocks=all

However, I'd consider providing a script that can invoke astyle
on just one file if needed.  You shouldn't require the user to
reformat all code in their hierarchy just to get a single new file.

Then you could include two scripts: one (let's call it A) that just
invokes astyle with the recommended options, and another that works like
your current script by invoking A via find+xargs -0 (as above).

Finally, I suggest that you use some combination of --pad and --unpad
to ensure that padding is normalized, too.  Probably these:

    --pad=oper
    --pad=paren-out
    --unpad=paren



More information about the Parted-edge mailing list