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?
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. I'd like to just remove this time bomb properly. -- Eli Schwartz Bug Wrangler and Trusted User