Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

single backslash \ at end of line could be disallowed within $'...' (and maybe others) #1825

Closed
bar-g opened this issue Feb 8, 2024 · 7 comments

Comments

@bar-g
Copy link
Contributor

bar-g commented Feb 8, 2024

Is it ok to pass the backslash (consumed by command line parsing?) on to the string literal, and it becoming a literal \ even though parse_backslash is off?

ysh ysh-0.19.0$ echo $'a\
> b'
a\
b
ysh ysh-0.19.0$

And this is a non-identical variation that is printing the same:

ysh ysh-0.19.0$ echo $'a\\
> b'
a\
b
ysh ysh-0.19.0$
@bar-g
Copy link
Contributor Author

bar-g commented Feb 9, 2024

It seems to be adding a backslash:

ysh ysh-0.20.0$ $'a\
> b'
  $'a\
  ^~
[ interactive ]:10: 'a\\\nb' not found
#                     ^^ why two?

Similar to here:

ysh ysh-0.20.0$ a'\'b
  a'\'b
  ^
[ interactive ]:9: 'a\\b' not found
#                    ^^ why two?

@bar-g
Copy link
Contributor Author

bar-g commented Feb 9, 2024

[ interactive ]:9: 'a\\b' not found may this actually rather mean:
[ interactive ]:9: $'a\\b' not found, i.e. having searched for r'a\b'?
(could make some sense as the prompt is saying ...$)

For the first example, I guess I would have expected it to fail with "Invalid char escape" (or print ab as with double-quotes)

The problem seems to be there is never an "invalid char escape" error, nor an escaped (dropped) newline (sameas in double-quoted strings) with no amount of backslashes before a newline.

@andychu
Copy link
Contributor

andychu commented Feb 9, 2024

It seems like this is identical to bash behavior

$ echo $'a\
> b'
a\
b


$ bin/ysh
ysh ysh-0.20.0$ echo $'a\
> b'
a\
b

The issue is that all strings in shell are MULTILINE strings. Single quotes, double quotes, etc.

\ at the end of the line is NOT an escaped newline -- that's only true outside quotes

@bar-g
Copy link
Contributor Author

bar-g commented Feb 9, 2024

What you say does not explain why the single \ is parsed with parse_backslash off.

It's also against what happens within double-quotes. There the newline (and the backslash before it) gets consumed, and no "Invalid char escape" is necessary:

ysh ysh-0.20.0$ echo "a\
> b"
ab
ysh ysh-0.20.0$

Consuming is consistent with shell:

$ string="
        one \
        two \
        three \
       "
$ echo "$string"

        one         two         three 

Plus python seems parsing single backslashes in singe- and double-quotes just fine, and there is still a multifold of ways remaining to denote the same string using \b.

This makes me ask how well backed up and necessary is the breaking behavior?

And maybe it does make sense, to rather improve uniqueness of denoting strings with a strict_ option forbidding singelton-backslash-pairs -- instead of singleton-backslashes -- to maintain compatibility with shell and python.

@andychu andychu changed the title escaped newline within $'...' single backslash \ at end of line could be disallowed within $'...' (and maybe others) Feb 9, 2024
@andychu
Copy link
Contributor

andychu commented Feb 9, 2024

Oh I see! Yeah actually that is a good point.

We want backslashes to always be doubled so there is one way of writing it

I'm not sure why it is the way it is

@andychu andychu added the strict label Feb 9, 2024
andychu pushed a commit that referenced this issue Feb 27, 2024
If

    $'foo\z'

is illegal, then this should be too:

    $'foo\
    line2
    '

The sequence \\ \n was a special case in _C_STRING_COMMON, giving
Id.Char_Literals.

I moved it down to lex_mode_e.SQ_C, and made it Id.Unknown_Backslash,
which means:

- It's literal in OSH (when parse_backslash is on)
- It's a syntax error in YSH (when parse_backslash is off)

This is issue #1825.
@andychu
Copy link
Contributor

andychu commented Feb 27, 2024

This is fixed - we now consistently disallow the single backslash then newline in YSH (details in commit description)

Will be out with the next release

Great testing, thank you!

@andychu
Copy link
Contributor

andychu commented Mar 22, 2024

@andychu andychu closed this as completed Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants