Skip to content

Conversation

@lens0021
Copy link
Contributor

@lens0021 lens0021 commented Oct 6, 2025

Reverts #732 and re-adds it in parse_escaped_string().

Test

echo "lorem ipsum doloret amat\x10"
trust $ echo \"Hello\" $
image

@lens0021 lens0021 requested a review from Ph0enixKM October 6, 2025 13:04
@lens0021 lens0021 self-assigned this Oct 6, 2025
@lens0021 lens0021 changed the title Create warnings when parsing Validate escape sequences when parsing Oct 6, 2025
Copy link
Member

@Ph0enixKM Ph0enixKM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lens0021 Thank you for your time and effort. This is a really good PR! Could you please add a compiler directive to disable this warning?

@Ph0enixKM
Copy link
Member

Ph0enixKM commented Oct 18, 2025

@lens0021 This is all good, but what about the rest of the escape sequences?

You can find all the escaped characters in the Bash's repository here in ansicstr located in lib/sh/strtrans.c. This function handles escape sequences. Here we can see that it handles: \a, \v, \b, \e, \E, \f, \n, \r, \t, octal (\0, \1, \2, \3, \4, \5, \6, \7), hexadecimal \x, unicode (\u, \U) and \c for interrupt.

Moreover, for any unrecognized escape sequences Bash copies literally (so for example \d remains \d in the string and not d).

Could you please add missing sequences and adjust the behavior? Since Bash doesn't complain about unrecognized escape sequences, we shouldn't either. Other commands utilize this behavior like sed for Regex where ( means a symbol of opening parentheses and \( means that it opens a group in Regex - even though \( is not a recognized sequence - other commands use this behavior.

@Ph0enixKM
Copy link
Member

@lens0021 now that I think about it. Do we actually need to display warning at all? Since there are so many edge cases and existing commands already abuse this Bash's behaviour of copy pasting literally the same escape sequence.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants