Optimize, Correct, and Improve LenChk #175
Conversation
* Prefer BASH way of redirecting both STDERR and STDOUT. * Remove unneeded `-f` flag from `type` builtin calls. * Replaced `readarray` in `Main()` with `while read` for BASH 3 users. Same operation, just supporting older versions of BASH, for those stuck on old versions of BASH on Mac or something. * Prefer `[[` keyword over `[` builtin. * Prefer arithmetic evaluation over `let` builtin. * Minimized `BASH_SOURCE` line, as irrelevant. Prior to this commit, LenChk used `readarray`, which was introduced in BASH 4.0. After testing LenChk in all complete versions from BASH 3.0 onwards, I found that LenChk required BASH >= 4.0 anyway. However, this commit introduces changes allowing it to work on as young as BASH 3.1. * Added BASH requirement (>= 3.1) to header. * Add common escape sequences as fallback if tput(1) not found. Targets the common terminal type we're probably all using. * Add missing `-n` flag from argument parsing block. Just reads more clearly, but seems to function the same. * Protected variable in array assignment. Addresses bug caused by directories with spacing or characters otherwise interpreted by the shell (IE: globbing) in their name. This wasn't apparent, but may have eventually been. * Remove unneeded syntax. * Corrected `Usage()`. Synopsis is now clearer. The flag arguments are also now correctly represented as required, by removing brackets around placeholders.
tests/lenchk
Outdated
| if [[ $BufferLine == '# cheat.sh: '* ]]; then | ||
| CheatLine=${BufferLine#'# cheat.sh: '} | ||
| IFS=',' read -a Fields <<< "$CheatLine" | ||
| for Field in "${Fields[@]}"; { | ||
| [ "$Field" == "$Progrm=disable" ] && SkipFile='True' | ||
| [[ $Field == $Progrm=disable ]] && SkipFile='True' |
There was a problem hiding this comment.
Strictly speaking you need quotes around $Program=disable, in the RHS part (but not around $Field!)
There was a problem hiding this comment.
Agreed. I wrote LenChk back before I had a brainwave moment, finally realising how inefficient and unnecessary [ is in BASH, these days.
There was a problem hiding this comment.
Hi. Finally getting around to this, having crawled back from my year-long hiatus.
Although word-splitting doesn't apply with variables in either side of [[, it is possible for someone to have glob pattern matching syntax in their path which leads up to the executable itself, which would be interpreted as such. However, $Progrm is ${0##*/}, leaving just the script name. Just in-case, because you never know what a user will do (lol), I will quote-protect it.
tests/lenchk
Outdated
| fi | ||
|
|
||
| # If the current file matches one which is whitelisted, skip it. | ||
| for CurWL in "${Whitelisted[@]}"; { | ||
| [ "$File" == "$CurWL" ] && continue 2 | ||
| [[ $File == $CurWL ]] && continue 2 |
There was a problem hiding this comment.
And here around $CurWL too (but not around $File !)
There was a problem hiding this comment.
I will do the same here for the same reason as mentioned above.
| # The line number of the problematic length. | ||
| [ "$NoColor" == 'True' ] || printf "$C_LineNums" | ||
| [[ $NoColor == True ]] || printf "$C_LineNums" | ||
| printf ' %7d ' $LineNum # <-- allows for 9,999,999 lines. |
There was a problem hiding this comment.
If it's strictly an integer, word-splitting will never apply.
|
I'll go over this PR again — bear with me. Hope you're well, BTW. :) |
|
Hi @terminalforlife |
|
Yeah, sorry for the lack of input. Don't worry about merging it, at least for now. I'll revisit this in the future. LenChk should be fine as-is, for the time being. Feel free to close the PR, if you're just trying to clean up the repository. I still have the changes locally for a future PR. |
* Quote-protected two variables to avoid obscure path weirdness. * Corrected embarrassing line number bug, caused by not resetting it. * Optimised and simplified `Usage()` by `printf`-ing just once. * Corrected unassigned `$BufferLine` variable to `$REPLY`. * Preferred POSIX character class to avoid locale issues. * Preassigned `$Whitelisted` array to ignore environment. The other changes were minor and superficial. Tested BASH versions -- still appears to be >= 3.1 supported.
|
I can only apologise for what was a crappy PR. Although I knew the original iteration of LenChk worked, I still left this here unfinished for a long time. I was burned out with Linux and programming, which ultimately led to a much needed break from it all. I've addressed the issues and brought it up to my current standards. I have tested it, but please do ensure you're happy with it first by testing it on your end as well. |
Please refer to commit message for all the information on the changes. Let me know if I missed something.