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