On Sun, 2020-03-15 at 10:58 +0900, John Crawley wrote:
> On 2020-03-14 13:27, jnq...@gmail.com wrote:
> > On Sat, 2020-03-14 at 14:59 +1100, David wrote:
> > > On Wed, 11 Mar 2020 at 02:39, <jnq...@gmail.com> wrote:
> > > > obviously it seems to be treating "syslinux,grub-efi" as a
> > > > single
> > > > package name which is wrong. this string originates from
> > > > LB_BOOTLOADERS.
> > > >
> > > > the code that should be handling this in source_debian looks to
> > > > be
> > > > the
> > > > following:
> > > > ```
> > > > echo "${LB_BOOTLOADERS}" | \
> > > > while IFS="," read -r BOOTLOADER
> > > > do
> > > > echo "${BOOTLOADER}" >> source-selection.txt
> > > > done
> > > > ```
> > > >
> > > > which is correctly specifying a comma as the separator, so if
> > > > this
> > > > is
> > > > where the problem originates, I don't know why...
> > >
> > > If you just need to parse comma-separated tokens from a string,
> > > then maybe this code will work for you:
> > >
> > > Demo code:
> > >
> > > #!/bin/sh
> > > s="a1,a2,a3,a4"
> > > while [ -n "${s}" ] ; do
> > > # get $v = leading word of $s, by strip first comma and all
> > > following
> > > v=${s%%,*}
> > > # update $s according to $v
> > > if [ "${v}" != "${s}" ] ; then
> > > # update $s by remove leading $v and a comma from $s
> > > s=${s#${v},}
> > > else
> > > # $v=$s, so nothing was stripped, so nothing more to do
> > > s=''
> > > fi
> > > printf 'v=%s\n' "${v}"
> > > done
> > >
> > > The above demo code tested produces output:
> > > v=a1
> > > v=a2
> > > v=a3
> > > v=a4
> >
> > Thankyou, I appreciate the effort you've put into this but an
> > explanation of why the existing implementation did not work was
> > supplied by John Crawley, and I went ahead already with an actual
> > solution of:
> > ```
> > for BOOTLOADER in $(echo "${LB_BOOTLOADERS}" | tr "," "\n"); do
> > echo "${BOOTLOADER}" >> source-selection.txt
> > done
> > ```
> >
> > which is ideally neat and simple, even if it is piping to a binary
> > rather than achieving the goal within the shell.
> >
> > your solution on the other hand, whilst I appreciate the effort put
> > into it, and possibly of interest to people to understand how you
> > accomplished it, is very much inferior imho considering how big and
> > complex it is relatively speaking. thank you again though.
>
> I think "very much inferior" might be overstating the case. David's
> pure
> shell implementation might look bigger, but it's probably faster,
> which
> might matter in some situations, and avoids calling outside
> utilities.
>
> This isn't a shell scripting mailing list, but FWIW here's yet
> another
> way. It's been tested on dash and posh, but does the work in a
> subshell
> which is fine if you just want to echo strings but won't allow you
> to
> set variables. It's a function:
>
> echo_strings()(
> local loader
> IFS=,
> set -- $*
> for loader
> do
> echo "$loader"
> done
> )
>
> LB_BOOTLOADERS='first bootloader,second one'
> echo_strings "$LB_BOOTLOADERS"
> first bootloader
> second one
>
> But the pipe to tr is probably fine for this case.Risking to state the obvious: if this snippet is ran once at setup, then keep it readable, short and maintainable. If it's in the "hot path" and ran a gazillion times per build, then by all means optimize away - but document it clearly. -- Kind regards, Luca Boccassi
signature.asc
Description: This is a digitally signed message part
