search

jcrodriguez-dis / moodle-mod_vpl

Virtual Programming Lab for Moodle (Module)
GNU General Public License v3.0
97 stars 84 forks source link

Allow whitespaces before and after <|-- and --|> (closes #168) #169

Closed Astor-Bizard closed 1 month ago

Astor-Bizard commented 4 months ago

This acts as backward compatibility / non-regressive robustness, as is was allowed before update 4.2.

jcrodriguez-dis commented 4 months ago

Dear Astor,

Thank you for bringing the whitespace sensitivity in block comments, introduced with update 4.2, to our attention. I understand that this change affects block comments parsing and may disrupt previously working activities by requiring block comments to start and end with <|-- and --|> without any surrounding whitespaces. Your suggestion to revert to the previous behavior to maintain compatibility with old test outputs is valid.

Regarding your proposed fix in #169, the choice between using [ \t] and \s in regex expressions to address this issue may be considered. The \s character class indeed matches whitespace characters including HT (9), LF (10), FF (12), CR (13), and space (32), and in certain locales, it may also match characters with code points in the range 128-255, including NBSP (A0). This broader match might not be ideal for ensuring the strict format of block comments where only horizontal tabs and spaces should be allowed as whitespace around <|-- and --|>.

We will review your proposal in detail. Our aim is to strike a balance between maintaining compatibility with existing test outputs and ensuring the robustness and clarity of block comments in our parsing logic.

Thank you again for your contribution and for helping us improve. We'll keep you updated on the progress of your proposed fix.

Best regards,

Juan Carlos

Astor-Bizard commented 4 months ago

You are right, I dit put the \s wildcard by default, but did not think this through. If you believe [ \t] is better, I'm fine with that (I don't see many cases where you might have vertical spaces there anyway... :D ) While we are on this question: do we want to allow whitespaces before single-line comments (e.g. Comment :=>>Some comment)?

Astor-Bizard commented 4 months ago

I added some commits, feel free to keep what you prefer! I changed the \s to [ \t] and allowed it before single-line comments as well.

Best regards, Astor