-
Notifications
You must be signed in to change notification settings - Fork 86
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
Add a marshalled output for index generation #1084
base: master
Are you sure you want to change the base?
Conversation
I do not like the |
I must admit I'm quite confused at the difference between *.odoc and *.odocl (are there others) ? |
@Drup In term of command line, you need a single module (cmti ?) to create a |
I think the idea is that In addition to the
Implementation and source trees are produced during the compile phase, and are meant to be linked. Contrary to this, occurrences and indexes are generated after the link phase, so I definitely agree that their extension should not be About the
So, I think we have to options:
I would prefer the first option. |
I know it is quite common in odoc, but at least for files names I really do not like it because it is a weird convention that fails to communicate its intent, so this requires knowledge of odoc to understand. It might also restrict names available for users, in a way that will be unexpected : no one will try to name a page I also do not like that file with the same extension are in fact of a different "type". I think you should be able to trust that the extension tells you what kind of file a file is, and in (1), all the Regarding implementation and sources, it seems to me that they are used in the same context (linking) and as such can share an extension. Their content could even be of the same ocaml type if we wanted to (maybe they are ?). I deeply favor (2), even though I understand that it requires changes out of the scope of this PR. The only benefits for (1) I found in your message is the fact it is done like that in other places in odoc, but as it seems insufficient with regard to the drawbacks expressed above. Maybe there are other benefits I did not consider ? |
The interface changes from the first commit are fine, sherlodoc only uses Fold.unit and Fold.page |
I'm not decided on which is better, but let me still answer to some of you comments
Using
This is not a problem at all. I don't think there can be any issue:
So:
|
(sherlodoc uses |
|
Yes, but it that case, we make up our own convention when a perfectly fine one already exists, so even though it's not unreasonable to ask people to learn things, my impression is that here, we are making it harder when its not necessary.
You are right that it will not cause conflicts, but it could be confusing for people who might be trying to understand what is happening looking at
Yes, I do not mean that they should be used completely interchangeably, this is not possible if two file have different content, but that there exists a context where they are all accepted. This is the case for json (you can always pass a json file to jq), and I hope it is for Regarding consistency, I think that this convention is more appropriate for To me, the main benefit of option (2) is that it makes it clear what you can do with the file, in a way that is understandable with minimal friction. This feel like a way more important information about a file than when it was produced. This is also more important because these files will used by tools external to odoc. I think I have expressed myself fully, maybe a third person could take the final decision if you still disagree after reading this. |
src/odoc/bin/main.ml
Outdated
in | ||
Arg.( | ||
value & opt_all convert_fpath [] | ||
& info ~doc ~docv:"FILE" [ "file-list" ]) | ||
in | ||
let marshall = | ||
let doc = "whether to output a json file, or an .odoc file" in |
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.
Small nitpick : I think it should be "a .odoc file" (pronounced "a dot odoc file")
I have read the diff of the second commit and it looks good to me aside from a very small nitpick. |
2861f69
to
f3919b0
Compare
3f5e2b9
to
205afd9
Compare
Standalone documentation comments currently do not have an id. This id was carried as the accumulator of the field, which yielded wrong results! Signed-off-by: Paul-Elliot <peada@free.fr>
Signed-off-by: Paul-Elliot <peada@free.fr>
205afd9
to
4d28c05
Compare
This PR adds a new marshalled output for the generation of index files.
This is desirable for several reasons:
The first commit fixes a bug in the computation of the ID of the page containing a standalone comment. The second commit is the actual change.
There are some design to discuss:
--marshall
flag to thecompile-index
: by default the output is json, but it is marshalled with this flag.index-
and have to have the.odoc
extension.But I think that is ok.