Skip to content

[Changes] Additional text for PR615#1452

Merged
damiendoligez merged 1 commit intoocaml:trunkfrom
rbonichon:format-changes-4.06
Oct 27, 2017
Merged

[Changes] Additional text for PR615#1452
damiendoligez merged 1 commit intoocaml:trunkfrom
rbonichon:format-changes-4.06

Conversation

@rbonichon
Copy link
Copy Markdown
Contributor

Follows from discussion of PR #1382.

A word for the changes required if you want to keep the old behavior as it was.

Follows from discussion of PR ocaml#1382.
Copy link
Copy Markdown
Member

@gasche gasche left a comment

Choose a reason for hiding this comment

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

Very nice, thank you!

@mshinwell
Copy link
Copy Markdown
Contributor

@rbonichon I noticed earlier this week that PR#615 has introduced a closure allocation every time a formatter's queue is flushed (because of the new clear_tag_stack function). This seems gratuitous but I'm not sure if it should be fixed for 4.06.

@gasche gasche added this to the consider-for-4.06.0 milestone Oct 27, 2017
@damiendoligez damiendoligez merged commit 6ba5866 into ocaml:trunk Oct 27, 2017
gasche pushed a commit that referenced this pull request Oct 27, 2017
Follows from discussion of PR #1382.
(cherry picked from commit 6ba5866)
@gasche
Copy link
Copy Markdown
Member

gasche commented Oct 27, 2017

Cherry-picked in 4.06 : af8f9e8.

@rbonichon
Copy link
Copy Markdown
Contributor Author

@mshinwell
Do you mean that in

let clear_tag_stack state =
  List.iter
    (fun _ -> pp_close_tag state ())
    state.pp_tag_stack

the (fun _ -> pp_close_tag state ()) is gratuitous ?

The type of _ here is actually string. The clear_tag_stack function was actually added to fix the tag closing behavior on flushes --- which was buggy.

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.

4 participants