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

MPR#7588: foreshadow features in format documentation #1291

Merged
merged 6 commits into from Aug 22, 2017

Conversation

Projects
None yet
4 participants
@Octachron
Copy link
Contributor

commented Aug 13, 2017

(see also MPR#7588)

This PR proposes to tweak the documentation of the Format module to make the existence of some features and functions more obvious:

First, it expands the preamble in order to introduce all major features of the Format module, i.e.

  • pretty-printing boxes
  • semantic tags
  • printf-like syntax
  • break hints
  • the existence of pp_ prefixed functions
  • formatters and predefined formatters

Without this change, the mere existence of some of these features (e.g. semantic tags or formatters) can be easily missed at first glance. For instance, the nominal group semantic tag does not pop up in the documentation before line 446.

Second, this PR pairs each generic pp_ prefixed function with its std_formatter counterpart. The aims of this reorganization are to make these functions much more visible and avoid a huge block of undocumented functions in the mist of the format documentation.

P.S.: Attentive readers may have noticed that tabulation boxes are untouched by this PR, this is due to the fact that tabulation boxes are currently missing their pp_ prefixed functions for some reason, I am therefore currently unsure of their current status.

@Octachron Octachron force-pushed the Octachron:doc_reorganize_format_foreshadowing branch from f63e8ee to 0d7e894 Aug 13, 2017

@Drup

Drup approved these changes Aug 14, 2017

Copy link
Contributor

left a comment

I agree with the basic idea of this PR. I think it leads itself to a much simpler presentation.

I also think we could even go further: present the printf functions first, in accordance to modern idioms with the %a combinators and use the description of syntax formats as a TOC for the rest of the document.

There are three predefined formatters:
- {!std_formatter} outputs to {{!Pervasives.stdout}stdout}
- {!err_formatter} outputs to {{!Pervasives.stderr}stderr}
- {!str_formatter} outputs to the {!stdbuf} {{!Buffer}buffer}

This comment has been minimized.

Copy link
@Drup

Drup Aug 14, 2017

Contributor

I'm not sure it's a good idea to advise usage of str_formatter. I think it's best to present the section about creating new formatters here directly.

This comment has been minimized.

Copy link
@Octachron

Octachron Aug 14, 2017

Author Contributor

Good point, I agree that str_formatter should not be here. I would rather keep for now the std,err_formatter part since it is needed in the current structure of the documentation to explain why all functions are duplicated.

For a gentle introduction to the basics of pretty-printing using
[Format], read
{{:http://caml.inria.fr/resources/doc/guides/format.en.html}
http://caml.inria.fr/resources/doc/guides/format.en.html}.
http://caml.inria.fr/resources/doc/guides/format.en.html}.

This comment has been minimized.

Copy link
@Drup

Drup Aug 14, 2017

Contributor

I think we could consider moving that whole paragraph inside the "boxes" subsection below.

This comment has been minimized.

Copy link
@Octachron

Octachron Aug 14, 2017

Author Contributor

Duly noted, I would keep that point in mind, but for a subsequent introduction tuning PR.

@darioteixeira

This comment has been minimized.

Copy link

commented Aug 14, 2017

Thumbs up! If there's one piece of the Stdlib in desperate need of better documentation is the Format module...

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2017

@Drup, I agree that the printf section should be put at the front of the documentation rather than in the backstage like currently. However, I find that these structure changes do not mix very well with more local changes. I am therefore thinking to bundle these changes in a separate structure reorganization PR.

@Drup

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2017

Yes, making these changes in separated bundles is for the best.

@gasche

gasche approved these changes Aug 21, 2017

Copy link
Member

left a comment

I haven't read the PR myself, but approve on the basis of Drup's review.

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2017

I have spotted an off-by-one error, I will merge after fixing this error.

@Octachron Octachron merged commit 1cb8443 into ocaml:trunk Aug 22, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.