-
Notifications
You must be signed in to change notification settings - Fork 211
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
Restore idempotency when formatting comments #1415
Conversation
This reduces the complexity of renderSrc slightly without affecting anything else much.
Fixes #1413. Previously the formatter would insert an additional line break before some comments whilst preserving existing line breaks. In order to restore idempotent behaviour, we need to strip a leading newline character from the comment string in those cases, if present.
Test case taken from @AJChapman's bug report (#1413).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers! :)
I think we should also try to test the idempotency of the formatting with a property test, but that could be done in a separate change.
let x = 1 in x | ||
-} | ||
renderSrc | ||
:: Maybe Src | ||
-- ^ Source span to render (if present) | ||
-> (Text -> Text) | ||
-- ^ Used to preprocess the comment string (e.g. to strip whitespace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that it would be more "conventional" to have the function as the first parameter, and the Maybe Src
as the second, but I'm not sure whether it's a good idea in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, now that you mention it, the ordering is a bit ad-hoc.
Good idea! |
Reads a bit more idiomatic, as suggested by @sjakobi.
Fixes #1413.
Previously we would repeatedly insert a newline character when formatting certain comments, e.g.
would result in (*)
which, when piped again through
dhall format
resulted inand so on.
With this change we only add a line break if the comment didn't already start with one (equivalently, we remove a leading line break before adding one back), meaning the formatter now stabilises at
in this case.