Fix parsing of strings with special characters

[MarikaChlebowska]

Description

If there were brackets in a string argument of a script it was always interpreted as interpolation before the change. That lead to unexpected outputs of such scripts. After this change arguments which are not intended as interpolation (not starting with $) and containing brackets will have implicitly added backticks for correct interpretation in the scripts. This fixes #10908.

To fix other issues mentioned in #11035 I changed the deparsing logic. Initially we added backticks for multi word variables and double quote if there was \ or " in the string. My change would add double quotes any time string starts with $ or contains any of character that might break parsing. The characters I identified are white space, (, ', `, ",and . It's possible other characters should be added to this list.

I tested this solution with few simple scripts using both stand alone arguments and flags and it seems to work but I would appreciate if someone with more experience checked it with some more unusual cases I missed.

User-Facing Changes

Erroneous behaviour described in the issue will no longer happen.

Tests + Formatting

Added tests for new formatting.

After Submitting

[fdncred]

I'm not following, brackets [] don't seem to be treated as string interpolation. I thought interpolation was only possible with $, single or double quote, text, and evaluated input between parenthesis like $"(ansi red)This is red(ansi reset)".

[MarikaChlebowska]

@fdncred
By brackets I meant parentheses (), sorry for confusion. As mentioned in the linked issue when an argument to a script is a string without spaces and with parentheses the parser will evaluate it as an interpolation. The cause I found is parsing (which removes any kind of quotation mark), then deparsing (which adds backtick if the string contains a space) and then it's parsed again and because of the lack of quotation for single word string the parse_string function interprets it as an interpolation due to following piece of code.

   // Check for bare word interpolation
    if bytes[0] != b'\'' && bytes[0] != b'"' && bytes[0] != b'`' && bytes.contains(&b'(') {
        return parse_string_interpolation(working_set, span);
    }

I assume there is a reason for this piece of code to stay as it is so I changed the deparsing logic to take into account the possibility of parentheses in a string.

[fdncred]

But with this PR, it still works the same the way it does in your linked issue.

❯ ./echo.nu "5"
5
❯ ./echo.nu "hi(1)"
hi1

❯ cat echo.nu                                                                                                                   34  03:39:27 PM
#!/usr/bin/env nu

def main [input: string] {
    print $input
}

[MarikaChlebowska]

It does. Are you sure you're running it using nu built on this branch? I got the following output

~/nushell> target/debug/nu echo.nu "5"                                                                                                                                                                                                                        11/11/23 23:19:29 PM
5
~/nushell> target/debug/nu echo.nu "hi(1)"                                                                                                                                                                                                                    11/11/23 23:19:36 PM
hi(1)
~/nushell> cat echo.nu                                                                                                                                                                                                                                        11/11/23 23:19:45 PM
def main [input: string] {
    print $input
}

[fdncred]

I could've messed up. Lemme check again.

[fdncred]

oh, I see what I did. I used your script which was the problem, LOL. It has a shebang that points to a different nu executable. You're right, it works now. I won't be landing this though because it's changing the parser. We need someone more experienced with it to approve these changes.

[MarikaChlebowska]

Marking this as draft since I might fix this issue together with #11035.

[MarikaChlebowska]

Reopening with fix for the second issue

[MarikaChlebowska]

One possible issue right now, my code might cause an error when parsing subcommand with a character that's not alphanumeric but I'm not sure what's the set of allowed characters in the subcommands to fix it

[MarikaChlebowska]

I changed my implementation again, I think now it shouldn't break anything

[fdncred]

let's give this a try

[fdncred]

i labeled breaking change because i suspect it will be one, not that i've seen it break anything yet.