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

Please document -ppx option #6135

Closed
vicuna opened this issue Aug 22, 2013 · 17 comments
Closed

Please document -ppx option #6135

vicuna opened this issue Aug 22, 2013 · 17 comments

Comments

@vicuna
Copy link

vicuna commented Aug 22, 2013

Original bug ID: 6135
Reporter: Hendrik Tews
Assigned to: @whitequark
Status: closed (set by @whitequark on 2017-05-18T10:55:56Z)
Resolution: fixed
Priority: urgent
Severity: minor
Version: 4.01.0+beta/+rc
Target version: 4.02.1+dev
Category: documentation
Monitored by: @whitequark @hcarty

Bug description

Looking at the 4.01beta reference manual I cannot find -ppx!

Apart from including -ppx it would be nice to have some basic information about ppx commands (howto read/write the syntax tree, the type of the syntax tree), as well as a pointer to more extensive documentation.

@vicuna
Copy link
Author

vicuna commented Aug 22, 2013

Comment author: @gasche

My personal hunch is that before we know what will be done with extension_points branch (eg. integrating it in time for a 4.02 release), there are relatively few legitimate uses of -ppx, and adding extension_points in the mix would probably require an extensive reformulation of the documentation if it exists. (There already is some documentation for extension_points specifically in http://caml.inria.fr/cgi-bin/viewvc.cgi/ocaml/branches/extension_points/experimental/frisch/extension_points.txt?view=markup ).

In particular, I think that using -ppx to overload the meaning of program fragments that already have a syntactic meaning in the existing OCaml language (as done in wmeyer's OMonad for experimentation purposes : http://danmey.org/omonad.html ) is a bad practice that we should not encourage. The question is what other use cases remain without extension_points. There are definitely some ( it is used by an experimental version of Bisect http://bisect.x9c.fr/index.html for example), but not as much as there could be in 4.02.

Of course more documentation is better than less, so if you or anyone feels motivated to write something (I guess the best thing to do for now is to look at the existing uses of -ppx to understand how to implement filters with it), I would be happy to have a look, provide feedback, and get something integrated in the manual if it's good.

@vicuna
Copy link
Author

vicuna commented Aug 22, 2013

Comment author: @damiendoligez

Hmm. I spend a whole day making sure that every command line option is documented, both in the manual and in the man pages, for ocamlc, ocamlopt, and ocaml...

As for giving a detailed documentation of the AST types, I think like Gabriel that it's probably too early.

@vicuna
Copy link
Author

vicuna commented Aug 23, 2013

Comment author: Hendrik Tews

OK, I didn't know that the extensions have not been merged yet.

Then I would suggest to briefly document -ppx with the calling convention and the data type, eg

-ppx Run as external filter on the
abstract syntax tree after parsing. The filter
is expected to read/write a marshaled ast
of type XXX on stdin/stdout. This option will
only get useful when extension points have been
implemented in one of the following releases.

@vicuna
Copy link
Author

vicuna commented Aug 20, 2014

Comment author: @mshinwell

Gabriel, what are we going to do about this? It's marked as target 4.02.0+dev.

@vicuna
Copy link
Author

vicuna commented Aug 20, 2014

Comment author: @damiendoligez

It's marked as target 4.02.0+dev.

That doesn't mean it must be done for 4.02.0, we can still postpone it. I talked to Alain in private, and he says we can (and should) document it after the release.

@vicuna
Copy link
Author

vicuna commented Aug 21, 2014

Comment author: @gasche

I don't know -ppx well enough to write the documentation (should I reassign the issue?). If Alain can do it, that's very good; otherwise we may try to nicely ask Peter "whitequark" Zotov for help, as he's played with -ppx a lot in the last few months.

@vicuna
Copy link
Author

vicuna commented Aug 21, 2014

Comment author: @damiendoligez

Alain is busy right now, so I'll release 4.02 without this documentation. It's still urgent but it can be written after the release.

@vicuna
Copy link
Author

vicuna commented Sep 27, 2014

Comment author: @damiendoligez

Any chance of getting this documented before 4.02.1 (i.e. within one week)?

@vicuna
Copy link
Author

vicuna commented Sep 27, 2014

Comment author: @whitequark

I'll send a PR within a few days. Should be quick enough for 4.02.1.

@vicuna
Copy link
Author

vicuna commented Sep 29, 2014

Comment author: @whitequark

Ok, so, I took a stab at it today.

First, the build system of ocaml-manual is ... let's say it needs improvements. It is not friendly to contributors right now.

Why does it try to use an OCaml from ../release? Why does it not set LD_LIBRARY_PATH properly for dllunix.so to be found? As a result it does not work with system compiler, or without (matching) system compiler.

There's also multind.sty which is not present even in texlive-extra from Debian. However this is easier to fix.

Second, and more importantly, compiler-libs are currently missing from the manual (and manpages aren't build as well). I was going to add a reference to Ast_mapper to the description of -ppx but it's not there... I'll take a look at how hard it is to add this to the manual.

@vicuna
Copy link
Author

vicuna commented Sep 29, 2014

Comment author: @lpw25

Second, and more importantly, compiler-libs are currently missing from the manual (and manpages aren't build as well). I was going to add a reference to Ast_mapper to the description of -ppx but it's not there... I'll take a look at how hard it is to add this to the manual.

I don't think we want to add compiler libs to the manual. They are not proper APIs and we make no guarantees about backwards-compatibility.

@vicuna
Copy link
Author

vicuna commented Sep 29, 2014

Comment author: @whitequark

lpw25: But compiler-libs, at least the parsing/ part, is required in order to write any ppx code. Its stability notwithstanding, this is what people are going to use anyway. I see no point in excluding those for ideological reasons; if something, the parsing/ APIs should be tidied up even more and made proper APIs.

I did some initial work in ocaml/ocaml-manual#5. More work on OCaml repo itself is required, specifically:

  • Update manpages.
  • Install manpages for Ast_mapper &co.
  • Explain how to implement rewriters in a few words at the top of Ast_mapper.mli.

@vicuna
Copy link
Author

vicuna commented Oct 4, 2014

Comment author: @whitequark

I added the documentation in OCaml as well, see #104.

Note that #6596 prevents it from being displayed properly in manpages.

@vicuna
Copy link
Author

vicuna commented Oct 7, 2014

Comment author: @gasche

Something that is slightly unsatisfying is that Alain's documentation comments in parsetree.mli are (1) using (* .. ) rather than (* ... *) and (2) are not formatted for ocamldoc rendering. This means that reading the source .mli is still the preferred way to use this file for documentation.

Would anyone volunteer to convert those comments into proper ocamldoc comments, so that they would show up in HTML documentations of the file?

@vicuna
Copy link
Author

vicuna commented Oct 7, 2014

Comment author: @gasche

I merged whitequark's changes in the documentation, but there is now a dangling reference in the HTML rendering of the compiler documentation (comps.etex) -- it works in the PDF rendering. We should still fix that.

@vicuna
Copy link
Author

vicuna commented Apr 5, 2017

Comment author: @Octachron

The dangling reference in the html version was fixed by #1129 .

@vicuna
Copy link
Author

vicuna commented May 16, 2017

Comment author: @mshinwell

Does anything more need doing here? It seems like the original problem has been fixed now, so I suggest we close this.

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

1 participant