bash-my-aws icon indicating copy to clipboard operation
bash-my-aws copied to clipboard

Improve efficiency and consistency of instance-ssh()

Open rawiriblundell opened this issue 2 years ago • 0 comments

Hi, as per the discussion in #324 , I'd like to propose the following changes to import-ssh():

  • Declare all local variables on a single line. This satisfies Shellcheck and reduces repeated local x=y visual clutter
  • Switch the usage output to use __bma_usage()
  • Rename user and USERNAME to ssh_user. This varname is more descriptive, and by avoiding UPPERCASE, we avoid a potential collision in the environment "scope"
  • Replace the multiple echo | awk sequences with a single read -r multiple variable assignment. This is the main efficiency improvement provided by this PR. This is, also, portable across the versions of ksh, bash and zsh that I've tested with.

I have increased the number of vars that use the optional brace syntax, but have not completely done so throughout. I personally prefer braces-by-default so that all vars, arrays, expansions etc have a consistent visual look. This is especially handy when you don't have syntax highlighting available, as the braces improves readability. Happy to complete 'the look' or revert based on your tastes @mbailey .

Potential code-golfing remains e.g.

  if [[ $1 != *i-* ]]; then
    ssh_user="${1}"
    shift
  fi

Could be something like:

[[ $1 != *i-* ]] && ssh_user="${1}"; shift 1

and

  if [[ -z "${instance_ids}" ]] ; then
    __bma_usage "[login] [instance-id] [instance-id]"
    return 1
  fi

Could be something like:

[[ -z "${instance_ids}" ]] && __bma_usage "[login] [instance-id] [instance-id]"; return 1

This second example would be consistent with other functions like instance-ssh-details().

rawiriblundell avatar Jul 29 '22 04:07 rawiriblundell