Skip to content

Fix bug where two contiguous ignore-begin blocks results in an error#367

Merged
michalmuskala merged 1 commit intoWhatsApp:mainfrom
ruippeixotog:fix-ignore-begin-end
Apr 25, 2025
Merged

Fix bug where two contiguous ignore-begin blocks results in an error#367
michalmuskala merged 1 commit intoWhatsApp:mainfrom
ruippeixotog:fix-ignore-begin-end

Conversation

@ruippeixotog
Copy link
Contributor

@ruippeixotog ruippeixotog commented Apr 25, 2025

Erlfmt currently can't parse two contiguous erlfmt:ignore-begin / erlfmt:ignore-end blocks. When we parse the first ignore-end we leave the formatter in an "end" state and don't clear it before the next "begin" since we don't run ignore_state_pre or ignore_state_post until the next non-comment. erlfmt doesn't handle transitions from end to begin, resulting in an error.

After checking the code, it's unclear to me why we need an end state in the first place - we always end up transforming it into false whenever we start or finish processing a statement. On this PR I'm simplifying things by simply setting the state to false after an erlfmt:ignore-end statement.

The unit test added fails in master and can be used to confirm the fix.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 25, 2025
@michalmuskala michalmuskala merged commit 510c046 into WhatsApp:main Apr 25, 2025
4 of 9 checks passed
@michalmuskala
Copy link
Member

Thank you! ❤️

The CI failing is due to ubuntu 20.04 deprecation, I'll fix that separately

@ruippeixotog ruippeixotog deleted the fix-ignore-begin-end branch April 25, 2025 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants