Skip to content

Conversation

@disaac
Copy link

@disaac disaac commented Jun 7, 2022

  • Added replace for bash variables of format ${variable} and $variable
  • Additional testing should be done to ensure that tab stop placeholder variables such as ${1:placeholder} are still properly handled. The replace is only looking for word characters after ${ and before } so shouldn't replace when : is present.
  • Tested with the following snippet text and verified output was as expected. Additional testing should be done to ensure placeholder
function blog() {
  local msg
  local logtype
  local loglevel
  local logfile txtblu txtgrn txtylw txtred txtrst thisfunc
  loglevel=4
  logfile="$(mktemp -t "$(basename "$0")" || exit 1)"
  logtype="$1"
  msg="$2"
  datetime="$(date +'%F.%H:%M:%S')"
  txtblu=$(tput setaf 12) # Blue
  txtgrn=$(tput setaf 10) # Green
  txtylw=$(tput setaf 11) # Yellow
  txtred=$(tput setaf 1) # Red
  txtrst=$(tput sgr0)
  thisfunc="${FUNCNAME[*]/blog/}"
  thisfunc="${thisfunc/main /}"
  logformat="[${logtype}]\t${datetime}\tfuncname: ${thisfunc}\t[line:$(caller 0 | cut -f 1 -d ' ')]\t${msg}"
  {
    case $logtype in
      debug)
        [[ $loglevel -le 0 ]] && echo -e "${txtblu}${logformat}${txtrst}"
        ;;
      info)
        [[ $loglevel -le 1 ]] && echo -e "${txtgrn}${logformat}${txtrst}"
        ;;
      warn)
        [[ $loglevel -le 2 ]] && echo -e "${txtylw}${logformat}${txtrst}"
        ;;
      error)
        [[ $loglevel -le 3 ]] && echo -e "${txtred}${logformat}${txtrst}"
        ;;
    esac
  } | tee -a "$logfile"
}

Issue: #68

* Added replace for bash variables of format `${variable}` and `$variable`

Issue:  pawelgrzybek#68
@pawelgrzybek
Copy link
Owner

Thank you for this contribution. Let me run your implementation against few different test cases manually and I will merge it. most likely after the weekend. Thank you again!

@disaac
Copy link
Author

disaac commented Jun 15, 2022

Thank you for this contribution. Let me run your implementation against few different test cases manually and I will merge it. most likely after the weekend. Thank you again!

@pawelgrzybek I actually found some bashisms that will still fail to be properly converted. I don't have time at the moment to PR on this but wanted to mention it since my PR would only address the cases mentioned in the issue:

echo "${someArray[@]}"
echo "${someArray[*]}"
settingDefaultValue="${settingDefaultValue:-"DefaultValue"}"

The above examples wouldn't be properly handled by the \w+ and would need to be addressed by another replace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants