Re: select syntax violates the POLA

[Greg Wooledge]

On Thu, Apr 01, 2021 at 11:58:13PM +0700, Robert Elz wrote:
> 
>   | If $1 is not a directory, then you want:
> 
> It is a directory, or I'd guess, quite likely a pattern chosen

It's amazing how many people manage to post their code with NO comments
or explanations of what it's supposed to do, what assumptions are being
made about the inputs, etc.  This leaves us to guess.

I'm guessing that /usr/src/pkg/*/* is a list of files, not a list of
directories.  You're guessing that it's a list of directories.

In the original code, if it's a list of directories, then we have
"ls firstdir seconddir" which expands to a list of mangled filenames
without directory components.  And since there's more than one directory,
the array ends up looking like:

d=(README foo.c foo.h foo README bar.c bar.h foo.c foo.h foobar)

with repeated filenames, and no knowledge of which filename came out of
which subdirectory.

I was assuming that this was *not* how it was intended to work, and thus,
I made the guess that /usr/src/pkg/*/$1 expanded to a list of *files*,
and therefore generated a list of full pathnames, e.g.

d=(/usr/src/pkg/applesauce/Makefile.PL /usr/src/pkg/bananabread/Makefile.PL)

and then the user can make a meaningful selection from such a list.

Now, this is obviously just a guess, and I could be extremely wrong
about it.  For one thing, they used the variable name "d".  That seems
to indicate that it should be a list of directories, rather than a list
of files.

So, perhaps their code is simply *broken*, and where they *thought* they
would be getting a list of directories, they actually get a list of
mangled useless filenames, because they forgot the -d argument to ls.

>   | d=(/usr/src/pkg/*/"$1")
> 
> definitely not that, the quotes are wrong in any case

They are not!

> (but apart from
> that, if filename expansion happens in array assignments (it doesn't in
> normal ones, and I dislike arrays, so ...) then without the quotes that
> might work.
> 
> Alternatively
>       d=( $( ls -d /usr/src/pkg/*/$1 ) )

This needs quotes around "$1" but yes, it's quite possible that this was
their intent.  Of course, this is still *broken*, but it only breaks if
one of the directory names contains whitespace characters.  It's likely
that they didn't happen to have any such directories in their test
environment, and therefore, this bug would go unnoticed for a long time.

> or just
>       d=( $( printf %s\\n /usr/src/pkg/*/$1 ) )

This is not much better.  It still breaks on whitespace.

If a list of directories (not their contents) is the desired outcome,
then my original code is perfectly fine:

d=(/usr/src/pkg/*/"$1")

This will expand to something like

d=(/usr/src/pkg/vendor1/netcat /usr/src/pkg/vendor2/netcat)

> Just to be sure.    Personally I'd do
> 
>       set -- /usr/src/pkg/*/$1
> 
> and then simply use the positional parameters.

This still needs quotes around "$1", and it's basically the same as
the array assignment -- just less flexible, and more suited to sh than
bash.  But certainly, this is viable in many cases.

> Yes.   But it turns out not to matter in this case, as none of
> the names will ever contain anything but "normal" characters
> (no spaces, newlines, asterisks, ...)

Famous Last Words™