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