fix: Sed improvements #1087
fix: Sed improvements #1087
Conversation
| plugins_path="$(get_plugin_path)" | ||
| # use find instead of ls -1 | ||
| # shellcheck disable=SC2012 | ||
| for plugin in $(ls -1 "$plugins_path" 2>/dev/null | sed "s#^$plugins_path/##"); do |
There was a problem hiding this comment.
I was able to remove both ls and sed from this line and replace the sed command with a simpler basename command. I'm considering adding ls to the banned commands list as I see it used in some anti-patterns in our codebase. To me it seems like listing files is done better with commands like find, grep, and the occasional globbing.
| ext_cmds=$( | ||
| ls -1 "$ext_cmd_path"/command*.bash 2>/dev/null | | ||
| sed "s#$ext_cmd_path/##" | ||
| ) |
There was a problem hiding this comment.
Same thing here. Was able to replace a ls and sed pipeline with just basename.
| local find_str=$2 | ||
| local replace=$3 | ||
| printf "%s" "${input//"$find_str"/"$replace"}" | ||
| } |
There was a problem hiding this comment.
A bit more code now, but this is much more robust than sed because it can handle any character in the three input strings and will handle escaping for us.
There was a problem hiding this comment.
Yes I believe so. But this is a Bash file and should only be executed by Bash.
Summary
Fixes: #686