[Bug] Variable inside a single apix string
Taking this example:
position = 1
$jq -r '.[0].assets.[{position}].browser_download_url'$
Generate this bash script:
jq -r '.[0].assets.[{position}].browser_download_url'
But inside a single apix string the variable doesn't get substituted, so Amber should report an error or compile it with double apix.
Looking at the issues with Shellcheck we have issues with variables that aren't wrapped with " are reported.
So I think that can be handy to:
- Remap all the strings with a single apix to a double one if there is a variable inside
- All the variables printed should be double apix wrapped
It seems a bit wrong since this would modify bash commands under the hood. The Bash that is written by user should verbatim be the same. Instead - this could be solved with the ORM you mentioned in another issue.
It is something that is language design, like the user can do whatever you want but if you use the std exposed by Amber automatically write Bash that is valid for shellcheck.
@Mte90 I've read this issue one more time and I take back what I said previously. This is a good issue. We should make the variables to be interpolated correctly even if they are in the verbatim string.
For what it's worth, here is a test.ab file I wrote.
// Output
// Succeeded
// Succeeded
// Succeeded
const txt = "Succeeded"
main {
trust $ echo {txt} $
trust $ echo "{txt}" $
trust $ echo '{txt}' $
}
Run result:
Succeeded
Succeeded
${__0_txt}
Built result:
__0_txt="Succeeded"
echo ${__0_txt}
echo "${__0_txt}"
echo '${__0_txt}'
I am thinking of this. I suggest that translating txt in $ echo '{txt}' $ to '${__0_txt}'. The first ' is for closing the outer, left quotation, and the second ' is for reopening the outer, right quotation. So the whole command should be echo ''${__0_txt}''. More examples here:
$ echo {txt} $→echo ${__0_txt}$ echo "{txt}" $→echo "${__0_txt}"$ echo '{txt}' $→echo ''${__0_txt}'', because the var_expr is in a single quoted text.$ echo "'{txt}'" $→echo "'${__0_txt}'"$ echo '"{txt}"' $→echo '"'${__0_txt}'"', same here$ echo '"'{txt}'"' $→echo '"'${__0_txt}'"'no, expr_var is not quoted.$ echo "\"{txt}"\" $→echo "\"${__0_txt}"\"
In this way, we can minimize the modification of the original bash code the user wrote. However, to do this, we should make the render_bash_value() function of var_expr know whether it is in a single quoted text. I don't know the compiler parses bash code. Does it, and could it be extended for this?
My above suggestion has a bug.
const txt = "Lorem ipsum" // contains a space
trust $ for v in '{txt}'; do echo \$v; done $
→ for v in '${__0_txt}'; do echo $v; done (bad, not interpolated at all)
→ for v in ''${__0_txt}''; do echo $v; done (bad, printing two lines)
It this case, the var_expr should be double-quoted between the empty single quotations.
→ for v in ''"${__0_txt}"''; do echo $v; done (good)
@lens0021 after my testings I found that the compiler parses the snippet correctly:
let position = 1
trust $jq -r '.[0].assets.[{position}].browser_download_url'$
Translates to:
__0_position=1
jq -r '.[0].assets.[${__0_position}].browser_download_url';
__AS=$?
What happened
Heraclitus parses this command region to the following form:
["$", "jq -r '.[0].assets.[", "{", "position", "}", "].browser_download_url'", "$"]
In order to solve the problem we must end the single quote string before the interpolation. This is how to do it:
$ cmd 'this is a string {var} something' $
# Should be translated to
$ cmd 'this is a string '{var}' something' $
This means that we need to check if according to the chunk of command before and after we are in an single_quote_context or not, and if so then we should end the region by wrapping the value with closing and ending just like in the example above. This is quite tricky though because the single quote does not always mean that it's a single quote string region. Here are some edge cases:
# These works properly:
echo "'$a'"
echo 'He said: '\''$a'\''!' # we need to handle escape symbols too
echo "`echo 'hi'`"
echo "$(echo 'hi')"
# This line has a ' but it does nothing
cat <<'END'
This is inside a single-quoted here-doc.
Single 'quotes' here won't start a new region.
END
Generally speaking to solve this properly we must accept that here anything could be written and if we start to support other shell targets, then the list of edge cases is going to grow. In my opinion we should close this issue and let the user close the single quote brackets. When they use a command syntax, then it's up to them to understand the bash code lying underneath.
This issue could be solved soon when we have nothing more important to do.
I understand your concerns for possibly growing edge cases.
Then, I have another idea. It is, without further parsing, wrapping the whole command with eval after escaping ".
trust $ echo "'$a'" $→eval "echo \"'${__0_a}'\""trust $ echo 'He said: '\''$a'\''!' $- I don't know this exclamation mark is used for the special meaning in Bash or just a mistake. Skipping now.trust $ echo "`echo 'hi'`" $→eval "echo "`echo 'hi'`""trust $ echo \"$(echo 'hi')\" $→eval "echo \"$(echo 'hi')\""trust $ # This line has a ' but it does nothing $→eval "# this line has a ' but it does nothing"-
This is not compiled at all. I don't remember Amber supports multi-line command.trust $ cat <<'END' This is inside a single-quoted here-doc. Single 'quotes' here won't start a new region. END $
Could you consider this too? @Ph0enixKM
I've tried my InterpolableRenderType::GlobalContext => format!("eval {quote}{result}{quote}") suggestion and discovered another edge case.
const s = "single quotes"
const s2 = "'{s}'"
trust $ echo '{s2}' $
$ amber build test.ab -
s="single quotes"
s2="'${s}'"
eval " echo '${s2}' "
$ amber run test.ab
single quotes
So I agree with this
In my opinion we should close this issue and let the user close the single quote brackets. When they use a command syntax, then it's up to them to understand the bash code lying underneath.
now completely.
Then, would we add a warning when single quote is used in command?
Hm, the above is maybe expected?
Sorry @lens0021 I didn't reply immediately but I wanted to tell you that I don't like this idea of wrapping every command into an eval context. This would further slow down the language and make the code even less readable.
@Mte90 This is an out of context question: Where the word apix is coined from? I couldn't find it anywhere.
- https://en.wikipedia.org/wiki/Apostrophe
- https://www.wikidata.org/wiki/Q436048
- Google search results
My bad I wrote wrong the word.