As Requested: Some Comments & Thoughts (Part 2) #2
Description
Part 1 is here: #1
Both head(1) and sed(1) here aren't needed. By default, the read builtin will already read the first line of its input, so there's your head(1) functionality sorted. As for sed(1), you can easily trim leading and trailing whitespaces with a parameter expansion's prefix and suffix removal, respectively. IE:
TrimSpaces() {
Str=$1
Len=${#Str}
for (( Index = 0; Index < Len; Index++ )); {
if [[ ${Str:Index:1} == ' ' ]]; then
(( Start++ ))
else
break
fi
}
for (( Index = Len - 1; Index > 0; Index-- )); {
if [[ ${Str:Index:1} == ' ' ]]; then
(( End-- ))
else
break
fi
}
printf '%s' "${Str:Start:End}"
}
printf '|%s|\n' "$(TrimSpaces ' here is a test ')"I used pipes to demonstrate the string barriers. I realise it might seem a bit unwieldy, but that, plus using just read to replace head(1), it's a more efficient use of system resources and you won't require sed(1).
Ideally, you'd:
Str=' here is a test '
Str=${Str## }
Str=${Str%% }
printf '|%s|\n' "$Str"Or use the [[ keyword with the =~ REGEX comparison operator and the $BASH_REMATCH array variable, but neither solution seemed to actually work, unfortunately. I'm not sure why. Perhaps I was having a blonde moment. The more manual approach works very well though.
https://github.com/The-Repo-Club/proctl/blob/4cd9e91dc413b720ff34579151617867298cebe8/proctl#L209
Again, you're using a substring unnecessarily, and what's more, is that you're probably exiting from the substring, making the exit status used here meaningless.
Are you trying to group commands together? You probably mean to use braces, like this:
{ echo 'Test 1'; echo 'Test 2'; } | grep 'Test'Notice how grep(1) matches both 'Test' lines? This approach groups commands together, which is very useful, but without using a subshell. Unnecessarily using subshells is inefficient. Just ask the Borg! :P
https://github.com/The-Repo-Club/proctl/blob/4cd9e91dc413b720ff34579151617867298cebe8/proctl#L231
Again, the double negatives confusing logic here need to be addressed. Refer to my first issue for one of my posts which explains this problem.
Again, using a subshell unnecessarily, when you probably want to just group them together with braces. Although, in this case, I recommend that you just an if statement. To some extent, it's a stylistic choice, so it's up to you.
You can replace tr(1) and head(1) here with something like:
Len=${#Str}
for (( Index = 0; Index < Len; Index++ )); {
Char=${Str:Index:1}
[[ $Char == ' ' ]] || Out+=$Char
}
Str=$OutBTW, that's an example of an unavoidable "double-negative", but it's still quite clear, I think. However, I've just realised I keep saying "double-negative", when that's not at all what this is. I'm not sure what an appropriate term would be, other than "confusing logic". Lol Hopefully you know what I mean, though.
You can avoid needing basename(1) by using parameter expansion's prefix removal, to clear everything up to and including the last '/' in a path. IE:
Path='/home/user/file.txt'
echo "${Path##*/}"https://github.com/The-Repo-Club/proctl/blob/4cd9e91dc413b720ff34579151617867298cebe8/proctl#L327
I'm not sure why you've written it in this way. :- means "set this to this if null", but you've chosen to set $EDITOR to null if it's null, which is a bit strange. Did you forget to finish that line? Lol
https://github.com/The-Repo-Club/proctl/blob/4cd9e91dc413b720ff34579151617867298cebe8/proctl#L334
I don't have and don't entirely know what fzf is, so I can't help much here, but I can say that I'm confident awk(1) and sed(1) can probably be replaced with a BASH approach here. I'm not sure how viable that is though, because I haven't got the original data and fzf. Remember, BASH's [[ keyword supports REGEX matching via =~, and remember that $BASH_REMATCH array variable is a thing for backreferencing.
You do this sort of thing a lot, I've noticed, which means there's probably a great opportunity for a function. You've got plenty of functions for the sake of functions, but not many functions for the sake of actually avoiding repetition and to make your code more flexible. To me, that's like buying car after car, but never ever driving them.
For example, search_license(), show_license(), language_picker(), get_language(), and initialize_repo() are used only once, so they're somewhat redundant. I realise there's a level of stylistic license here, but I can't not mention it. Lol Your approach would make more sense in C, I imagine.
However, if it truly helps you program effectively, then ignore me.
I'm not entirely sure why you've used awk(1) here, because, if it were needed, it'd break the depends call anyway. Actually, I'm pretty sure a space in a filename is not allowed or is particularly non-standard in $PATH. Am I missing something here?
https://github.com/The-Repo-Club/proctl/blob/4cd9e91dc413b720ff34579151617867298cebe8/proctl#L512
Unfortunately, due to the way you've checked for dependencies (when they're needed), it's gonna be a massive pain if someone is missing a bunch. They'll run the program, be told they need something, likely in the middle of it actually doing a bunch of stuff, which is potentially problematic, then they correct that, only to run the program and find they're missing something else! Check any one of my programs (AE, for example) and you'll see that I handle all dependencies first, before I do anything that requires an external tool. There's almost always a method to my madness. Lol
There are other things I could point out, but I'd pretty much be repeating myself, and I need a break; maybe I need a function for this. ;)
I think that's it done. Overall, I think you're doing really well. :) Give me a shout if you need me to look at anything else. Also, sorry this didn't make it to a video, if that's what you had in mind. In retrospect, I probably should've just done that. Lol