4 ms·

Before releasing a script, it's always a good idea to fix the errors reported by Shellcheck. There are several here. Also the script has a /bin/sh shebang and R

Before releasing a script, it's always a good idea to fix the errors reported by Shellcheck. There are several here. Also the script has a /bin/sh shebang and README says it's POSIX but it has bashims (for example "echo -e" will fail in default /bin/sh in Debian).

Hello71 5y ago
there are more issues after fixing all of those: grep -w is not POSIX, the exit status of type is not clearly defined by POSIX, realpath is not POSIX, basename is POSIX but can be replaced with ${path##*/} for most well-formed paths (gives different results for /, but this script doesn't work properly for / anyways?), inodes are not unique across devices, --directory=dir has ugly handling but seems to work but --directory dir doesn't (i think it silently sets the directory to --directory and then passes dir as a separate argument?)
- wurosh 5y ago
  I fixed everything you mentioned. I kept basepath because it handles more edge cases. I improved the option parsing to handle --directory dir, although that is neither here nor there (it's GNU.. -ish). The handling is ugly for the sake of POSIX compatibility - if you can think of a more elegant way to write it let me know, I'd be glad to incorporate it. The exit status of type is clearly defined by POSIX (https://pubs.opengroup.org/onlinepubs/9699919799/utilities/type.html https://pubs.opengroup.org/onlinepubs/9699919799/utilities/t...). Either way, thank you for the exhaustive list of problems, it really helped - please don't hesitate to open issues with any further observations.
  - Hello71 5y ago
    more specifically the problem with type is it's not clear whether "An error occurred" includes the case where the command is not found. one could reasonably argue that printing "command not found" is a successful run. additionally, it's only present in the XSI extension. for these reasons, command -v is usually preferred for better POSIX compatibility.
wurosh 5y ago
I usually have it on. Turns out I didn't install the binary on this PC which was silently ignored by my editor. Fixed! Thanks for pointing it out!

Hello71 5y ago

there are more issues after fixing all of those: grep -w is not POSIX, the exit status of type is not clearly defined by POSIX, realpath is not POSIX, basename is POSIX but can be replaced with ${path##*/} for most well-formed paths (gives different results for /, but this script doesn't work properly for / anyways?), inodes are not unique across devices, --directory=dir has ugly handling but seems to work but --directory dir doesn't (i think it silently sets the directory to --directory and then passes dir as a separate argument?)

wurosh 5y ago
I fixed everything you mentioned. I kept basepath because it handles more edge cases. I improved the option parsing to handle --directory dir, although that is neither here nor there (it's GNU.. -ish). The handling is ugly for the sake of POSIX compatibility - if you can think of a more elegant way to write it let me know, I'd be glad to incorporate it. The exit status of type is clearly defined by POSIX (https://pubs.opengroup.org/onlinepubs/9699919799/utilities/type.html https://pubs.opengroup.org/onlinepubs/9699919799/utilities/t...). Either way, thank you for the exhaustive list of problems, it really helped - please don't hesitate to open issues with any further observations.
- Hello71 5y ago
  more specifically the problem with type is it's not clear whether "An error occurred" includes the case where the command is not found. one could reasonably argue that printing "command not found" is a successful run. additionally, it's only present in the XSI extension. for these reasons, command -v is usually preferred for better POSIX compatibility.

wurosh 5y ago

I usually have it on. Turns out I didn't install the binary on this PC which was silently ignored by my editor. Fixed! Thanks for pointing it out!