ocaml / odoc

Documentation compiler for OCaml and Reason
Other
322 stars 88 forks source link

Improvement in rendering of hidden substitutions #961

Open dbuenzli opened 1 year ago

dbuenzli commented 1 year ago

The whole experience for writers improved quite a bit over the years. Basically we no longer have to define artificial types we can now write:

module M : sig 
  type t

  module Sub : sig 
    (**/**)
    type m := t
    (**/**) 

    type t 
    val get_m : t -> m
  end
end

And at render time that m reference is turned into a t that links to M.t. However from a reading perspective it can get confusing:

Screenshot 2023-05-13 at 14 48 22

That second link goes to M.t.

Should maybe odoc render the fully qualified name in this case ?

For reference here's what happens if one does not hide the substitution, it's then quite clear what's happening but m links on the substitution and another name is still introduced:

Screenshot 2023-05-13 at 14 53 56
jonludlam commented 1 year ago

Would a tooltip with the complete path help? I'd rather something we can apply universally than just attempt to detect types that render the same but point to different targets (would we do the ambiguous check per value, per signature, per include?)

dbuenzli commented 1 year ago

Would a tooltip with the complete path help?

Doesn't really help with reading, I can already hover on the link and it shows me the destination. Basically for now I opted not to hide substitutions, it gets too confusing otherwise.

I'd rather something we can apply universally than just attempt to detect types that render the same but point to different targets (would we do the ambiguous check per value, per signature, per include?)

The idea would be to apply it on any hidden substitution regardless of whether it renders the same as something else.

panglesd commented 1 week ago

I'm not sure using a fully qualified name would work here. There is always a chance this fully qualified name also exists as a relative name:

module M : sig 
  type t

  module Sub : sig 
    module M : sig
      type t
    end

    (**/**)
    type m := t
    (**/**) 

    type t 
    val get_m : t -> m
  end
end

Neither t nor M.t can replace m here without confusion...

I don't think we can name a shadowed identifier in a regular name. Maybe, t/2 or something could be used?

But still, I think that keeping the type m := t visible is the most readable.