[arch-projects] [dbscripts] [PATCH 1/2] Don't quote $PKGEXT

[Luke Shumaker]

On Thu, 15 Feb 2018 16:57:26 -0500,
Eli Schwartz wrote:
> 
> On 02/15/2018 04:43 PM, Luke Shumaker wrote:
> > That's not a bad idea.  But then someone reading the code might wonder
> > "why does such a trivial function exist?".  I think it would be silly,
> > and ultimately hurt readability to go through and replace all
> > 
> > "[[ -f ... ]]" instances with "file_exists", and if we don't do that,
> > then there's a weird magic question of "when should I use [[ -f ]] and
> > when should I used file_exists?"
> 
> Why would it hurt readability?
> 
> Why won't people be able to read the comments I wrote documenting the
> function in my working tree?

Every Bash programmer knows what [[ -f ]] does.  Sure
 - "file_exists" is self-explanatory, and
 - a comment there might better explain why it exists instead of using [[ -f ]]
But then it's another silly little thing that they have to keep in
mind everywhere in the codebase.

What about just adding a comment:

    # We can't use [[ ]] for these 2 checks because glob expansion
    # needs to be performed on $PKGEXT.

It avoids someone tripping over it in the future, and avoids adding
another unnecessary abstraction to the codebase.

> > Ultimately, this way doesn't hide anything, and has everything the
> > next person needs to know right there.  Sure, they'll have to be
> > careful not to trip.  The next patch I sent changes it to PKGEXT_glob,
> > to make it stand out a little more.  I'm also working on a `make
> > lint`/shellcheck patchset that will add `# shellcheck disable=SC2086`
> > directives to each line to avoid shellcheck complaining about
> > PKGEXT_glob being unquoted, drawing even more attention to it, to
> > avoid tripping.
> 
> I want to make dbscripts more readable, not less. Just reverting every
> new thing when *both* are broken, is not something I want to do.

Huh?  My version isn't broken.  Unless you mean that the glob is more
restrictive than the one used by makepkg?

-- 
Happy hacking,
~ Luke Shumaker