[PATCH 1/3] email-sh-functions: make merge addresses more flexible

[Keller, Jacob E]

On Wed, 2014-02-12 at 12:01 +0200, Artem Bityutskiy wrote:
> On Tue, 2014-02-11 at 09:50 -0800, Jacob Keller wrote:
> > This patch combines effort from a previous patch, as well as worm from
> > Artem to make merge_addresses more flexible. First, it enables multiple
> > line inputs, so that it can convert a list of addresses per line into a
> > single comma seperated string. In addition, make the arguments optional,
> > so it will merge as many argumements as it is passed instead of limiting
> > to 1 or 2.
> > 
> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy at linux.intel.com>
> > Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> > ---
> >  email/aiaiai-email-sh-functions |   29 +++++++++++++++++------------
> >  1 file changed, 17 insertions(+), 12 deletions(-)
> > 
> > diff --git a/email/aiaiai-email-sh-functions b/email/aiaiai-email-sh-functions
> > index 5474577..f0c5807 100644
> > --- a/email/aiaiai-email-sh-functions
> > +++ b/email/aiaiai-email-sh-functions
> > @@ -8,6 +8,9 @@
> >  . shell-ini-config
> >  . shell-quote
> >  
> > +__br="
> > +"
> > +
> >  if [ -z "${__included_aiaiai_email_sh_functions-}" ]; then
> >  __included_aiaiai_email_sh_functions=1
> >  
> > @@ -100,16 +103,22 @@ fetch_project_name()
> >  	printf "%s" "$list" | LC_ALL=C sed -n -e "s/.*$l+\([^@]\+\)@$d.*/\1/p" | head -n1
> >  }
> >  
> > -# Merge e-mail addresses into a comma-separated list
> > -# Usage: merge_addresses "addr1" "addr2"
> > +# Merge e-mail addresses into a comma-separated list. This function may have
> > +# any number of arguments. Each input argument is allowed to contain multiple
> > +# lines of comma separated e-mail addresses. This function will merge them all
> > +# into a single line of comma separated e-mail addresses.
> > +# Usage: merge_addresses "addr1" .. "addrN"
> >  merge_addresses()
> >  {
> > -	local addr1="$1"; shift
> > -	local addr2="$1"; shift
> > -	local list="$(printf "%s" "$addr1,$addr2" | LC_ALL=C tr -d "\n")"
> > +	local list
> > +
> > +	# We set IFS inside the subshell to be a newline, so that expansion of
> > +	# $* will properly add blank lines. This enables the trim to replace
> > +	# newlines with commas.
> > +	list="$(IFS="${__br}"; printf "%s" "$*" | LC_ALL=C tr "\n" ",")"
> >  
> 
> Looks good, thanks.
> 
> Just one tiny nit-pick.
> 
> My patter for this project is:
> 
> IFS="$__br" command
> 
> which changes IFS _only_ for command, but not globally.
> 

That doesn't workf or a shell builtin. We're inside a subshell though,
so shouldn't that not change for the whole command?

> As opposed to
> 
> IFS="$__br"; command
> 
> which changes it globally.
> 
> In your case this does not matter since you run this in a subshell
> ("$()"), but... this is more of a methodology... let me explain.
> 
> Shell is very ugly language. It allows for writing horrible cruft,
> easily.

I understand. I tried this first, but it didn't work.

> 
> The only way to write more or less maintainable scripts is to stick to
> good practices and good patterns. And consistently use the patterns all
> over. Only _then_ shell scripts  become more or less readable. Of
> course, the reader first has to grasp the patterns, though :-)
> 
> So please, let's stick to the pattern I used :-)
> 
> I'll amend this line, if you do not mind.

It won't work, because printf is a builtin, and won't recognize the
change in IFS if it's in the same statement. :( I tried this way first
and it failed.

> 
> Thanks!
> 

Regards,
Jake