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

fix: foldLine escape character logic #2

Merged
merged 1 commit into from
May 27, 2024

Conversation

johnhooks
Copy link

@johnhooks johnhooks commented May 27, 2024

The escape character logic was not working correctly.

The logic of how a line is folded with a final escape character was flawed. Using a loop to consume back slashes would not allow for an escaped back slash to be folded correctly, it would append an extra character to the line.

  • Use if statements to check for escape characters. Only one extra character should be appended to the line, and it should be considered escaped.
  • Add new test cases to test the escape character logic.

The escape character logic was not working correctly.

The logic of how a line is folded with a final escape character was
flawed. Using a loop to consume forward slashes would not allow for an
escaped forward slash to be folded correctly, it would append an extra
character to the line.

- Use if statements to check for escape characters. Only one extra
  character should be appended to the line, and it should be considered
  escaped.
- Add new test cases to test the escape character logic.
@johnhooks johnhooks requested a review from erikyo May 27, 2024 12:38
@@ -134,7 +137,7 @@ export function foldLine (str, maxLen = 76) {
if ((match = /.*\s+/.exec(curLine)) && /\S/.test(match[0])) {
// use everything before and including the last white space character (if anything)
curLine = match[0];
} else if ((match = /.*[\x21-\x2f0-9\x5b-\x60\x7b-\x7e]+/.exec(curLine)) && /[^\x21-\x2f0-9\x5b-\x60\x7b-\x7e]/.test(match[0])) {
} else if (!escaped && (match = /.*[\x21-\x2f0-9\x5b-\x60\x7b-\x7e]+/.exec(curLine)) && /[^\x21-\x2f0-9\x5b-\x60\x7b-\x7e]/.test(match[0])) {
Copy link
Author

@johnhooks johnhooks May 27, 2024

Choose a reason for hiding this comment

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

I don't know if this is the right thing to do, but this was matching the backslash and I don't know why. We need more information about what this condition should be checking for, and figure why its matching a backslash character.

Copy link
Author

Choose a reason for hiding this comment

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

I don't know why this should match.

'a\\b'.length;
// => 3
match = /.*[\x21-\x2f0-9\x5b-\x60\x7b-\x7e]+/.exec('a\\b');
match[0];
// => 'a\\'

Examination of the escapes as Buffers.

Buffer.from('\\');
// <Buffer 5c>
Buffer.from('\x21-');
// <Buffer 21 2d>
Buffer.from('\x2f0');
// <Buffer 2f 30>
Buffer.from('\x5b-');
// <Buffer 5b 2d>
Buffer.from('\x60');
// <Buffer 60>
Buffer.from('\x7b-');
// <Buffer 7b 2d>
Buffer.from('\x7e');
// <Buffer 7e>

Copy link

@erikyo erikyo May 27, 2024

Choose a reason for hiding this comment

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

yes I agree also because it is not clear what this code is for, I think in order not to break the lines maybe if there is a consecutive \ and \n immediately after it?

/.*[\x21-\x2f0-9\x5b-\x60\x7b-\x7e]+/.exec('a\\b')
match because
/.*
because this match the 'a'
[\x21-\x2f0-9\x5b-\x60\x7b-\x7e]+
and this the \ and the + matches the previous token between one and unlimited times

@erikyo erikyo merged commit dd91ced into wp-blocks:tests/shared May 27, 2024
5 checks passed
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