[Parted-edge] Patches waiting for bindings.git

[Otavio Salvador]

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 :(

> +astyle --mode=c \
> +       --indent=spaces=4 \
> +       --indent-classes \
> +       --indent-switches \
> +       --indent-cases \
> +       --indent-namespaces \
>
> Does --indent-namespaces make sense with --mode=c ?

I hadn't check again but I think I did it when looking the diffs produced.

> +       --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

Improved, check...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: enforcing-coding-style_infra
Type: application/octet-stream
Size: 2674 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/parted-edge/attachments/20070511/3e4aa88e/enforcing-coding-style_infra.obj
-------------- next part --------------


-- 
        O T A V I O    S A L V A D O R
---------------------------------------------
 E-mail: otavio at debian.org      UIN: 5906116
 GNU/Linux User: 239058     GPG ID: 49A5F855
 Home Page: http://otavio.ossystems.com.br
---------------------------------------------
"Microsoft sells you Windows ... Linux gives
 you the whole house."