utterly broken code everywhere

[zendeavor]

sorry to be a doomsayer.

for starters, this file is full of function definitions that work based on pure luck alone
https://github.com/revans/bash-it/blob/master/lib/preexec.bash
defining a function with "function name ()" is broken, unportable, and not guaranteed to work across bash versions. see http://mywiki.wooledge.org/BashPitfalls#pf25

here is an example of inconsistency in function definitions https://github.com/revans/bash-it/blob/master/lib/history.bash

and this file has mixed definitions https://github.com/revans/bash-it/blob/master/lib/helpers.bash

you're writing bash-specific code; use the tools to your advantage. single-bracket test ( [ ... ] ) is fully discouraged when writing scripts with a bash shebang. use double-bracket test ( [[ ... ]] ) instead -- ESPECIALLY since you don't properly quote your substitutions and expansions ever. expansions within [[ .. ]] are not subject to word-splitting which saves you the headache of writing good code in some situations. refer to http://mywiki.wooledge.org/tests and http://mywiki.wooledge.org/WordSplitting for more information.

https://github.com/revans/bash-it/blob/master/lib/composure.sh
in this file, you defined transcribe() twice. redundant. you also define a function to echo some keywords, which you later re-use to compose other functions to execute `:' through eval. now you can get away with the misappropriated use of eval here thanks to your finite & known input but you really shouldn't be doing this at all. first, composure_keywords() shouldn't be a function. it should be an array, expanded with ''for keyword in "${composure_keywords[@]}"; do ...; done''

also i'm pretty positive you meant "composition" rather than "composure" unless erichs thought this play on words was unbelievably clever.

https://github.com/revans/bash-it/blob/master/lib/composure.sh#L18 see ''help typeset.'' (unless you're super concerned about your scripts being portable to ksh...wot)

https://github.com/revans/bash-it/blob/master/lib/composure.sh#L57
shopt -s nocaseglob case $input in (y[es]*) ...;; (n[o]) ... ;; esac

https://github.com/revans/bash-it/blob/master/lib/composure.sh#L143
greybot │ basename(1) can strip the directory and extension from a path (a/b/c.d -> c.d or c). A parameter expansion can do it cleaner and much faster: "${path##/}", "${path%.}"
** you should probably replace every occurence of the triply expensive pipe to sed with a parameter expansion.

greybot │ Use pgrep instead of ps to get PIDs. It saves you from having to parse output that wasn't meant to be parsed. However, finding a process by its name is unsafe. Please look at http://mywiki.wooledge.org/ProcessManagement
****** please note the comment "it saves you from having to parse output ----that wasn't meant to be parsed----"

uhmmmm, wtf is this https://github.com/revans/bash-it/blob/master/bash_it.sh#L13
the backticks create an expensive subshell in the first place. and in that subshell, you launch a THIRD bash instance just to ''echo $BASH'' and this whole construct is 100% equivalent to "var=$BASH" exactly like you have in the line before. even worse though, is that BASH is set by default already and you've just been wasting these cpu cycles for god knows how long. see ''declare -p BASH'' in a shell that isn't running bash-it.

okay, i will just leave it here. if you fix all of these problems, you'd have solved about 60% of the issues that will crop up over time due to the horrible syntactic constructs employed herein.

[tswicegood]

Thanks for taking the time to document these issues. bash-it is used by myself and dozens (hundreds, thousands?) of people every day and these issues haven't stopped our use. That said, I'm sure there's tons of room for improvement. Bash is not a language I'm intimately familiar with, so I'm sure there's tons of code that I've created that's not anywhere near optimal.

I'm happy to review and merge any pull requests you'd like to open to address these concerns. This is an open source project, so it only gets better when people like yourself who are better coders than the original team lend a hand in cleaning up and enhancing the software.

[zendeavor]

i'd argue there are too many corrections/patches to make for a simple pull-request for merging, and the project would be better served if the owners fixed these up over a period of time rather than reviewing and merging 10 or more pull requests from myself (or one large "i fixed it all" request)

[tswicegood]

@zendeavor sorry, but that's not the way open-source works. Feel free to submit any patches you like. In the meantime, I'm closing this issue as wontfix. Everything you outlined works fine for everyone else that's using the project.

Like I said yesterday, if you're interested in getting your hands dirty and helping fix the problem you perceive instead of painstakingly documenting those problems, I'm happy to review and merge anything that makes the project better. Code speaks louder than bug reports that simply outline problems.

[Spaceghost]

For what it's worth, it's a nice little collection of scripts that ostensibly work pretty well. Pragmatism in action.