[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Cleanup, simplifications and bugfixes in scripts



Hi Florian,

all in all, the patch is nice. I have some questions, for which I don't
have a (best) answer yet.

On Thu, Jun 20, 2013 at 10:15:53PM +0200, Florian Bruhin wrote:
> -VERSIONARGS=( $(tr . ' ' <<< "$VERSION") )
> +IFS=. read -a versionargs <<< "$version"

Nice, I didn't knew, this is possible. :)

>  herbstclient --idle '(tag_changed|goto_last_tag|reload)' \
>      | while read line ; do
> -        ARGS=( $line )
> +        args=( $line )

Can we also use this read -a thing here?

> -hc=${herbstclient_command:-herbstclient}
> +hc="${herbstclient_command:-herbstclient}"

The quotes should not be needed here, isn't it?

>  simple_command() {
> -    arg=$($hc complete 1 "$@"|dmenu_cmd -p "$@:") \
> -    && exec $hc "$@" "$arg"
> +    arg=$("$hc" complete 1 "$@" | dmenu_cmd -p "$@:") \
> +    && exec "$hc" "$@" "$arg"

> -$hc complete 1 use |
> +"$hc" complete 1 use |
>  while read tag ; do
> [..]
> -    $hc layout "$tag" \
> +    "$hc" layout "$tag" \

> [similar hunks...]

My idea was to allow default arguments in herbstclient (maybe there will
be a flag like --use-this-and-that-protocol in the future). So the
decision is: Do we allow default arguments for herbstclient or allow
spaces in the path of the herbstclient binary... (That's exactly the
reason why hlwm-commands are arrays of strings and not just strings.)


> -function uniq_linebuffered() {
> -    awk -W interactive '$0 != l { print ; l=$0 ; fflush(); }' "$@"
> +uniq_linebuffered() {
> +    awk -W interactive '$0 != l { print ; l=$0 ; fflush(); }' "$@" 2>/dev/null
>  }

Why do you want to ignore stderr of awk? There are some versions of awk
that do not support fflush(), so it may be useful if panel.sh would
complain about that on stderr.

Regards,
Thorsten



Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev