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

Renderers: allow for unqualified identifier rendering #636

Open
dbuenzli opened this issue Mar 12, 2021 · 6 comments
Open

Renderers: allow for unqualified identifier rendering #636

dbuenzli opened this issue Mar 12, 2021 · 6 comments
Labels

Comments

@dbuenzli
Copy link
Contributor

I'm sure this is going to be controversial but I'll put it in to see what the mood is.

In general odoc html rendered specifications are much less readable than the .mli they were generated from. The reason is that we get all the prefixes from the namespacing modules they belong to. This is precise but effectively, in a given context, these tend to become noise.

I'd like to suggest to render identifiers mostly as they are found in the .mli, that is strip the open modules at most until the first prefix (so that we never get fully unqualified identifiers except for the built-in types).

Following the links allows to make clear exactly what the identifier is about and optionally we could use a name attribute so that a tooltip reveals the fully qualified identifier.

I'm sure people are going to suggest we could put that under a cli flag. I suggest we do not do so because in general when you devise docs you need a fair idea on how things are going to be presented.

A few things that are related to that:

@Drup
Copy link
Contributor

Drup commented Mar 12, 2021

First off: it's not specific to the HTML renderer. All those decisions are taken when translating the model into the document IR, so it's common to all renderer.

Now, I mostly agree with you. We need to be careful about shadowing though. The tooltip idea is a good one.
It does require some model changes to keep track of scopes, which might not be so easy.

@dbuenzli dbuenzli changed the title HTML renderer: unqualified identifier rendering Renderers: allow for unqualified identifier rendering Mar 12, 2021
@lpw25
Copy link
Contributor

lpw25 commented Mar 12, 2021

We've always intended to include opens in the output and then shorten the names accordingly. I'm a bit nervous about doing the shortening without also including the opens.

Whilst it is definitely an ad-hoc rule, I'd be happy to use your suggestion for all open! whilst including the open in the docs for all other opens. That would work extremely well with Jane Street's style.

@avsm
Copy link
Member

avsm commented Mar 25, 2021

I do think the opens need to be included in the output if the names are stripped, but that's a good opportunity in the output to cross-reference to the module being opened as the reader goes through the interface...

@dbuenzli
Copy link
Contributor Author

I'd still like #594 be solved regardless of opens being rendered (note this is not about signature). It's quite annoying at the moment since I have to qualify a lot of my references that didn't need it before – and I'd rather not have that qualification in the end.

@trefis
Copy link
Contributor

trefis commented May 30, 2022

I'd like to suggest to render identifiers mostly as they are found in the .mli, that is strip the open modules at most until the first prefix (so that we never get fully unqualified identifiers except for the built-in types).

That sounds nice, but I don't think it would work well in presence of includes.
Consider this artificial example:

  • foo.mli and bar.mli both containing a type t
  • a.mli:
    open Foo
    
    val something: t -> int -> int
    
    module Consumer : sig
     val consume_foo : t -> unit
     val consume_bar : Bar.t -> unit
    end
    
    (* ... *)
  • b.mli:
    open Bar
    
    val something : t -> int -> int
    
    include Foo.Consumer (** @inline *)

What would the doc of B look like?

I think if we want something nice, we're going to have to keep track of currently opened modules, and do the shortening ourselves.
(That ought to be much easier than the compiler's -short-path work though)

@dbuenzli dbuenzli added output enhancement New feature or request labels Jun 9, 2022
@github-actions
Copy link

github-actions bot commented Aug 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. The main purpose of this is to keep the issue tracker focused to what is actively being worked on, so that the amount and variety of open yet inactive issues does not overwhelm contributors.

An issue closed as stale is not rejected — further discussion is welcome in its closed state, and it can be resurrected at any time. odoc maintainers regularly check issues that were closed as stale in the past, to see if the time is right to reopen and work on them again. PRs addressing issues closed as stale are as welcome as PRs for open issues. They will be given the same review attention, and any other help.

@github-actions github-actions bot added the stale label Aug 7, 2023
@dbuenzli dbuenzli added not stale and removed stale labels Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants