fix: incorrect variable name display in must_get_env error msg#22615
fix: incorrect variable name display in must_get_env error msg#22615mdqst wants to merge 1 commit into
must_get_env error msg#22615Conversation
jakubgs
left a comment
There was a problem hiding this comment.
I don't really see any issue with that function as it is right now in develop:
~/work/status-mobile develop 51s
> bash
[jakubgs@lilim:~/work/status-mobile]$ eval "$(sed -n '/function must_get_env/,+8p' scripts/build-android.sh)"
[jakubgs@lilim:~/work/status-mobile]$ must_get_env TEST
No required env variable: TEST
exit
|
jakubgs hey, thanks for checking! the problem is that in the error message you use If you run: unset TEST_VAR
must_get_env TEST_VARthe error shows without the name. Changing it to just echo -e "${RED}No required env variable:${RST} ${BLD}$1${RST}" 1>&2that way it shows: |
| set -e | ||
|
|
||
| GIT_ROOT=$(cd "${BASH_SOURCE%/*}" && git rev-parse --show-toplevel) | ||
| GIT_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")" && git rev-parse --show-toplevel) |
There was a problem hiding this comment.
How is that better?
is better in terms of clarity, correctness, and consistency, especially in more complex scripts or when the script could be sourced.
| return | ||
| fi | ||
| echo -e "${RED}No required env variable:${RST} ${BLD}${!VAR_VALUE}${RST}" 1>&2 | ||
| echo -e "${RED}No required env variable:${RST} ${BLD}${VAR_NAME}${RST}" 1>&2 |
There was a problem hiding this comment.
The ! trick works because VAR_VALUE is not a variable but rather a reference to a var created via declare -n:
-n make NAME a reference to the variable named by its value
Which means that VAR_VALUE is a reference to $1. We can test this:
> TEST=something
> VAR_NAME=TEST
> declare -n VAR_REF=$VAR_NAME
> echo $VAR_REF
something
> echo ${!VAR_REF}
TESTYour version is clearer, but the result is the same. What I would like to understand is why for you it doesn't expand into the name of variable from the nameref.
noticed a bug in the
must_get_envfunction where the error message was showing the value reference (${!VAR_VALUE}) instead of the actual variable name (${VAR_NAME}).fixed it so that the error now correctly displays the name of the missing environment variable.