kmr-srbh commented 5 months ago

Fix:

2490 gave wrong output due to the incorrect handling of the escape sequence '\f'. Escaping the sequence was handled, but unescaping was missed. This led to the string `" \t\n\v\f\r"` being transformed to `" \t\n\v\\f\r"` in the AST. Note the extra "\" after "v".

Screenshot from 2024-02-21 17-30-11

Hence, the answer in #2490 was thus 'not actually' wrong as non-whitespace and lowercase characters did exist. Fixes #2490

Improvement

Though the error in the above case was due to incorrect string handling, the isspace() method was actually not implemented correctly. The definition of a 'whitespace' character is actually broad. The Python interpreter checks for all of them in it's implementation of the method. I incorporated checking for those characters.

A huge shout-out to @advikkabra for the clever testing. :tada:

Note

The Black formatter has automatically formatted the Python file I worked on. Please note that only change I made was the implementation of the isspace() method.

Last but not the least, I thoroughly enjoyed working on the issue. :smiley:

Thirumalai-Shaktivel commented 4 months ago

What is the status of this PR?

kmr-srbh commented 4 months ago

What is the status of this PR?

It is ready for review @Thirumalai-Shaktivel.

Thirumalai-Shaktivel commented 4 months ago

Please mark this PR ready for review once it is ready.

lcompilers / lpython

Fixes #2490: Adds unescaping `\f` and the correct implementation of `isspace()` string method. #2551

Fix:

2490 gave wrong output due to the incorrect handling of the escape sequence '\f'. Escaping the sequence was handled, but unescaping was missed. This led to the string `" \t\n\v\f\r"` being transformed to `" \t\n\v\\f\r"` in the AST. Note the extra "\" after "v".

Improvement

Note

lcompilers / lpython

Fixes #2490: Adds unescaping `\f` and the correct implementation of `isspace()` string method. #2551

Fix:

2490 gave wrong output due to the incorrect handling of the escape sequence '\f'. Escaping the sequence was handled, but unescaping was missed. This led to the string " \t\n\v\f\r" being transformed to " \t\n\v\\f\r" in the AST. Note the extra "\" after "v".

Improvement

Note

2490 gave wrong output due to the incorrect handling of the escape sequence '\f'. Escaping the sequence was handled, but unescaping was missed. This led to the string `" \t\n\v\f\r"` being transformed to `" \t\n\v\\f\r"` in the AST. Note the extra "\" after "v".