[arch-projects] [dbscripts] [PATCH 1/2] Don't quote $PKGEXT
lukeshu at lukeshu.com
Thu Feb 15 22:17:13 UTC 2018
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?
~ Luke Shumaker
More information about the arch-projects