Skip to content
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

Conversion specifications and pretty-priinting indications #6418

Closed
vicuna opened this issue May 13, 2014 · 7 comments
Closed

Conversion specifications and pretty-priinting indications #6418

vicuna opened this issue May 13, 2014 · 7 comments
Assignees

Comments

@vicuna
Copy link

vicuna commented May 13, 2014

Original bug ID: 6418
Reporter: @diml
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2015-12-11T18:28:05Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.0+dev
Fixed in version: 4.02.0+dev
Category: standard library
Monitored by: @yakobowski

Bug description

This code:

Format.fprintf ppf "@{<c %d>" 42

used to be interpreted as:

Format.pp_open_tag ppf "c 42"

now it is interpreted as:

Format.pp_open_tag ppf "c %d"

If it's not too hard to support the old behavior I think it would be worth it.

File attachments

@vicuna
Copy link
Author

vicuna commented Jun 6, 2014

Comment author: bvaugon

So, I attach two patches (and instructions to apply them and bootstrap) that seems to resolve the problem.

@vicuna
Copy link
Author

vicuna commented Jun 6, 2014

Comment author: @mshinwell

Gabriel, are these ok?

@vicuna
Copy link
Author

vicuna commented Jun 6, 2014

Comment author: @gasche

I haven't done a full review yet but I've been discussing this with Benoît and I think it is. Which is important, as it would be our last standing format-related regression.

@vicuna
Copy link
Author

vicuna commented Jun 9, 2014

Comment author: @gasche

I reviewed and merged Benoît's patch in both 4.02 and trunk.

There is still a known regression with the following variant that we still do not support:

Format.printf "@[<hov %d>foo@\nbar@\nbaz@]" 4

It should not be very hard to add this on top of Benoît's solution -- which was itself non-trivial.

@vicuna
Copy link
Author

vicuna commented Jun 11, 2014

Comment author: bvaugon

Please find two attached patches fix-open-box-patch-{1,2}.diff that fix the "@[<%s %d> @]" problem. The bootstrap procedure is similar to the one for fix-open-tag-patch-?.diff patches.

@vicuna
Copy link
Author

vicuna commented Jun 11, 2014

Comment author: bvaugon

The fix-open-box-patch-3.diff fix a bug when the indentation is not specified (as in "@[", for example).

@vicuna
Copy link
Author

vicuna commented Jun 14, 2014

Comment author: @gasche

Thanks a lot! I merged those patches in 4.02 and trunk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants