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

[Luke Shumaker]

On Thu, 15 Feb 2018 15:11:13 -0500,
Dave Reisner wrote:
> 
> On Thu, Feb 15, 2018 at 02:49:45PM -0500, Luke Shumaker wrote:
> > -	[[ -f ${FTP_BASE}/${PKGPOOL}/${pkgname}-${pkgver}-${pkgarch}${PKGEXT} ]] && return 1
> > -	[[ -f ${FTP_BASE}/${PKGPOOL}/${pkgname}-${pkgver}-${pkgarch}${PKGEXT}.sig ]] && return 1
> > +	[ -f "${FTP_BASE}/${PKGPOOL}/${pkgname}-${pkgver}-${pkgarch}"${PKGEXT} ] && return 1
> > +	[ -f "${FTP_BASE}/${PKGPOOL}/${pkgname}-${pkgver}-${pkgarch}"${PKGEXT}.sig ] && return 1
> 
> Rather than making this stand out like a sore thumb for the next person
> to trip over, why don't we just define a "file_exists" function?
> 
>   file_exists() {
>     [[ -f $1 ]]
>   }
> 
> Now you're free to do this:
> 
>   file_exists "${FTP_BASE}/${PKGPOOL}/${pkgname}-${pkgver}-${pkgarch}"${PKGEXT} && return 1
> 
> Expansion is taken care of by the shell before you pass to exists, and
> the check does what you'd expect.

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?"

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.

-- 
Happy hacking,
~ Luke Shumaker