Remove tyxml from odoc_html_frontend #1072

EmileTrotignon · 2024-01-17T13:29:34Z

Also include a bugfix : name with no prefix are not displayed with a . in front.

Removing Tyxml is interesting for the frontend : it results in smaller executable.
As the typing is different, in an exposed library, it would be beneficial for this to be stabilized quickly.

Also include a bugfix : name with no prefix are not displayed with a `.` in front.

panglesd · 2024-01-17T13:38:26Z

How much difference is there in the executable size, when produced in "release" mode? I would have though tyxml is not too big, given that it is very often used client-side.

trefis · 2024-01-17T13:39:12Z

Removing Tyxml is interesting for the frontend : it results in smaller executable.

A note about the context here: this lib is compiled to js when used by sherlodoc (so it results in smaller js files).

EmileTrotignon · 2024-01-17T13:48:23Z

The difference is more than 10k. I do not think it is huge, and would be fine with it, but Arthur disagrees.
Besides, the important part is the interface, and the new interface is still compatible with tyxml.

EmileTrotignon · 2024-01-17T13:50:07Z

As for why tyxml is that big, it forces uses of format which is a big library.

panglesd · 2024-01-17T13:51:36Z

Besides, the important part is the interface, and the new interface is still compatible with tyxml.

I'm not sure what you mean here!

The difference is more than 10k.

For a bit of precision, 10k out of how much?

EmileTrotignon · 2024-01-17T13:52:50Z

It was actually ~108k out of 212k which is a big improvement.

EmileTrotignon · 2024-01-17T13:54:24Z

Besides, the important part is the interface, and the new interface is still compatible with tyxml.

I'm not sure what you mean here!

The current interface locks us into using Tyxml. With mine we can use anything, and change the implementation later, without having to change sherlodoc.

It will be important for usability in the future not to have too much constraints on which odoc version is compatible with which sherlodoc version.

Drup · 2024-01-17T14:24:18Z

As for why tyxml is that big, it forces uses of format which is a big library.

Mmh, I remember it wasn't so hard to avoid pulling Format for Tyxml_js in the past, but that's not something I track carefully on the tyxml side. Maybe I should find a way to enforce it 🤔.

(hello all 👋 )

panglesd

At the same time, it's nice not to force the consumer of the library to use tyxml, at the same time, it's nice to provide the data in a structured form rather than a string.

Also, re-implementing tyxml in odoc_html_frontend.ml makes future modification cumbersome.

However, if tyxml adds a lot to the output (in its current form), and given that it's unlikely that search engine will really use these "data in a structured form" soon, I'm OK with this PR (though, not very enthusiastic!)

panglesd · 2024-01-17T15:07:49Z

src/search/odoc_html_frontend.ml

  and prefix_name =
    match prefix_name with
+    | None -> []
+    | Some "" -> []
    | Some prefix_name ->
        [ span ~a:[ a_class [ "prefix-name" ] ] [ txt (prefix_name ^ ".") ] ]


There is indeed a bug here. But wouldn't it be better to remove the addition of "." to the prefix name:

Suggested change

and prefix_name =

match prefix_name with

| None -> []

| Some "" -> []

| Some prefix_name ->

[ span ~a:[ a_class [ "prefix-name" ] ] [ txt (prefix_name ^ ".") ] ]

and prefix_name =

match prefix_name with

| None -> []

| Some prefix_name ->

[ span ~a:[ a_class [ "prefix-name" ] ] [ txt prefix_name ] ]

and modify names_of_id accordingly?

Yes, it is better i agree.

On second thought, no its not better. I believe there a few place where we String.split the prefix name. This would behave very poorly

In sherlodoc or in odoc? Could you provide links to such places?

That would maybe mean that the prefix name is turned into a string too early, and we should keep it as a list?

Its in sherlodoc.
We should probably keep it as a list.

I went and checked more closely. We do indeed use String.split on the name, but we do not use the name functions provided by odoc.search, we call fullname which is a list, so it does not change anything to switch that around. We could even say that the bug was sherlodoc's fault, we could have given None instead of Some "".

EmileTrotignon · 2024-01-17T15:35:53Z

The ci was failing on formating, so I called dune fmt. It made a diff in an unrelated file for reasons I ignore.

CHANGES.md

@jonludlam

CHANGES: ### Highlight - Hierarchical documentation (@jonludlam, @panglesd, @Julow) Pages can now be organized in a directory tree structure. Relative and absolute references are added: `{!./other_page.label}`, `{!//other_page}`. - Improved sidebar and breadcrumbs navigation (@panglesd, @gpetiot) The documentation pages and the libraries of the entire package are shown on the left sidebar. - Added support for images, videos, audio and other assets The syntax is `{image!/reference/to/asset}` or `{image:URL}` for images. The syntax for `{video...}` and `{audio...}` is the same. (@panglesd, @EmileTrotignon, ocaml/odoc#1170, ocaml/odoc#1171, ocaml/odoc#1184, ocaml/odoc#1185) - Search using Sherlodoc (@panglesd, @EmileTrotignon, @Julow) A new search bar that supports full-text and type-based search. ### Added - Experimental driver (@jonludlam, @panglesd) The driver builds the documentation for a collection of Opam packages using the newer Odoc features. It supports linking external packages to ocaml.org and markdown files. This is experimental and will break in the future. - Cross-package references (@panglesd, @Julow) Pages and modules from other packages can be referenced: `{!/otherpackage/page}`, `{!/otherpackage/Module.t}`. - Option to remap links to other packages to ocaml.org or other site. See the `--remap` option of the driver or the `--remap-file` option of `odoc html-generate`. (@jonludlam, ocaml/odoc#1189, ocaml/odoc#1248) - Option to compute occurrences of use of each identifiers The commands `aggregate-occurrences` and `count-occurrences` are added. (@panglesd, ocaml/odoc#976, ocaml/odoc#1076, ocaml/odoc#1206) - Added the `odoc classify` command (@jonludlam, ocaml/odoc#1121) Helps driver detecting which modules belong to which libraries. - Added `--suppress-warnings` to the CLI to remove warnings from a unit, even if they end up being raised in another unit through expansion (@jonludlam, ocaml/odoc#1260) - Add clock emoji before `@since` tag (@yawaramin, ocaml/odoc#1089) - Navigation for the search bar : use '/' to enter search, up and down arrows to select a result, and enter to follow the selected link. (@EmileTrotignon, ocaml/odoc#1088) - Fix a big gap between the preamble and the content of a page (@EmileTrotignon, ocaml/odoc#1147) - Add a marshalled search index consumable by sherlodoc (@EmileTrotignon, @panglesd, ocaml/odoc#1084) - Allow referencing of polymorphic constructors in polymorphic variant type aliases (@panglesd, ocaml/odoc#1115) - Added a home icon in the breacrumbs (@panglesd, ocaml/odoc#1251) It can be disabled with a CLI option. - Add a frontmatter syntax for mld pages (@panglesd, ocaml/odoc#1187, ocaml/odoc#1193, ocaml/odoc#1243, ocaml/odoc#1246, ocaml/odoc#1251) Allows to specify the title of a page, the order of sub-pages and other behaviors in the sidebar. - Added `odoc-md` to process standalone Markdown pages (@jonludlam, ocaml/odoc#1234) ### Changed - The command line interface changed to support the new features. + Packages and libraries: `odoc link` must now be aware of packages and libraries with the `-L libname:path` and `-P pkgname:path` options. The module search path should still be passed with the `-I` option. The current package should be specified with `--current-package=pkgname`. + Hierarchy: `odoc compile` now outputs `.odoc` in the directory tree specified with `--output-dir=DIR` and the parent identifier must be specified with `--parent-id=PARENT`. The option `--source-parent-file` is removed. + Source code: Implementations are compiled with `compile-impl` instead of with `compile`. The options `--cmt=..` and `--source-name=..` are removed. Source code pages are generated with `html-generate-source`. + Assets: The commands `compile-asset`, `html-generate-asset` are added. The option `html-generate --asset` is removed. + Sidebar: The index is built using `compile-index`. The sidebar data is extracted from the index with `sidebar-generate` and passed to `html-generate --sidebar=..`. - The syntax for `@tag` is now delimited (@panglesd, ocaml/odoc#1239) A `@tag` can now be followed by a paragraph or other elements. - Updated colors for code fragments (@EmileTrotignon, ocaml/odoc#1023) - Fixed complexity of looking up `.odoc` files (@panglesd, ocaml/odoc#1075) - Normalize whitespaces in codespans (@gpetiot, ocaml/odoc#1085) A newline followed by any whitespaces is normalized as one space character. - Reduce size of `Odoc_html_frontend` when compiled to javascript (@EmileTrotignon, ocaml/odoc#1072) - Overhaul of module-type-of expansions and shadowing code (@jonludlam, ocaml/odoc#1081) - Output file paths and labels in the man and latex backends changed to avoid name clashes (@Julow, ocaml/odoc#1191) ### Fixed - Fix variant constructors being hidden if they contain hidden types (@jonludlam, ocaml/odoc#1105) - Fix rare assertion failure due to optional parameters (@jonludlam, ocaml/odoc#1272, issue ocaml/odoc#1001) - Fix resolution of module synopses in {!modules} lists that require --open (@jonludlam, ocaml/odoc#1104} - Fix top comment not being taken from includes often enough (@panglesd, ocaml/odoc#1117) - Fixed 404 links from search results (@panglesd, ocaml/odoc#1108) - Fixed title content not being picked up across pages when rendering references (ocaml/odoc#1116, @panglesd) - Fix wrong links to standalone comments in search results (ocaml/odoc#1118, @panglesd) - Remove duplicated or unwanted comments with inline includes (@Julow, ocaml/odoc#1133) - Fix bug where source rendering would cause odoc to fail completely if it encounters invalid syntax (@jonludlam ocaml/odoc#1208) - Add missing parentheses in 'val (let*) : ...' (@Julow, ocaml/odoc#1268) - Fix syntax highlighting not working for very large files (@jonludlam, @Julow, ocaml/odoc#1277)

@jonludlam

CHANGES: - Hierarchical documentation (@jonludlam, @panglesd, @Julow) Pages can now be organized in a directory tree structure. Relative and absolute references are added: `{!./other_page.label}`, `{!//other_page}`. - Improved sidebar and breadcrumbs navigation (@panglesd, @gpetiot) The documentation pages and the libraries of the entire package are shown on the left sidebar. - Added support for images, videos, audio and other assets The syntax is `{image!/reference/to/asset}` or `{image:URL}` for images. The syntax for `{video...}` and `{audio...}` is the same. (@panglesd, @EmileTrotignon, ocaml/odoc#1170, ocaml/odoc#1171, ocaml/odoc#1184, ocaml/odoc#1185) - Search using Sherlodoc (@panglesd, @EmileTrotignon, @Julow) A new search bar that supports full-text and type-based search. - Experimental driver (@jonludlam, @panglesd) The driver builds the documentation for a collection of Opam packages using the newer Odoc features. It supports linking external packages to ocaml.org and markdown files. This is experimental and will break in the future. - Cross-package references (@panglesd, @Julow) Pages and modules from other packages can be referenced: `{!/otherpackage/page}`, `{!/otherpackage/Module.t}`. - Option to remap links to other packages to ocaml.org or other site. See the `--remap` option of the driver or the `--remap-file` option of `odoc html-generate`. (@jonludlam, ocaml/odoc#1189, ocaml/odoc#1248) - Option to compute occurrences of use of each identifiers The commands `aggregate-occurrences` and `count-occurrences` are added. (@panglesd, ocaml/odoc#976, ocaml/odoc#1076, ocaml/odoc#1206) - Added the `odoc classify` command (@jonludlam, ocaml/odoc#1121) Helps driver detecting which modules belong to which libraries. - Added `--suppress-warnings` to the CLI to remove warnings from a unit, even if they end up being raised in another unit through expansion (@jonludlam, ocaml/odoc#1260) - Add clock emoji before `@since` tag (@yawaramin, ocaml/odoc#1089) - Navigation for the search bar : use '/' to enter search, up and down arrows to select a result, and enter to follow the selected link. (@EmileTrotignon, ocaml/odoc#1088) - Fix a big gap between the preamble and the content of a page (@EmileTrotignon, ocaml/odoc#1147) - Add a marshalled search index consumable by sherlodoc (@EmileTrotignon, @panglesd, ocaml/odoc#1084) - Allow referencing of polymorphic constructors in polymorphic variant type aliases (@panglesd, ocaml/odoc#1115) - Added a home icon in the breacrumbs (@panglesd, ocaml/odoc#1251) It can be disabled with a CLI option. - Add a frontmatter syntax for mld pages (@panglesd, ocaml/odoc#1187, ocaml/odoc#1193, ocaml/odoc#1243, ocaml/odoc#1246, ocaml/odoc#1251) Allows to specify the title of a page, the order of sub-pages and other behaviors in the sidebar. - Added `odoc-md` to process standalone Markdown pages (@jonludlam, ocaml/odoc#1234) - The command line interface changed to support the new features. + Packages and libraries: `odoc link` must now be aware of packages and libraries with the `-L libname:path` and `-P pkgname:path` options. The module search path should still be passed with the `-I` option. The current package should be specified with `--current-package=pkgname`. + Hierarchy: `odoc compile` now outputs `.odoc` in the directory tree specified with `--output-dir=DIR` and the parent identifier must be specified with `--parent-id=PARENT`. The option `--source-parent-file` is removed. + Source code: Implementations are compiled with `compile-impl` instead of with `compile`. The options `--cmt=..` and `--source-name=..` are removed. Source code pages are generated with `html-generate-source`. + Assets: The commands `compile-asset`, `html-generate-asset` are added. The option `html-generate --asset` is removed. + Sidebar: The index is built using `compile-index`. The sidebar data is extracted from the index with `sidebar-generate` and passed to `html-generate --sidebar=..`. - The syntax for `@tag` is now delimited (@panglesd, ocaml/odoc#1239) A `@tag` can now be followed by a paragraph or other elements. - Updated colors for code fragments (@EmileTrotignon, ocaml/odoc#1023) - Fixed complexity of looking up `.odoc` files (@panglesd, ocaml/odoc#1075) - Normalize whitespaces in codespans (@gpetiot, ocaml/odoc#1085) A newline followed by any whitespaces is normalized as one space character. - Reduce size of `Odoc_html_frontend` when compiled to javascript (@EmileTrotignon, ocaml/odoc#1072) - Overhaul of module-type-of expansions and shadowing code (@jonludlam, ocaml/odoc#1081) - Output file paths and labels in the man and latex backends changed to avoid name clashes (@Julow, ocaml/odoc#1191) - Fix variant constructors being hidden if they contain hidden types (@jonludlam, ocaml/odoc#1105) - Fix rare assertion failure due to optional parameters (@jonludlam, ocaml/odoc#1272, issue ocaml/odoc#1001) - Fix resolution of module synopses in {!modules} lists that require --open (@jonludlam, ocaml/odoc#1104} - Fix top comment not being taken from includes often enough (@panglesd, ocaml/odoc#1117) - Fixed 404 links from search results (@panglesd, ocaml/odoc#1108) - Fixed title content not being picked up across pages when rendering references (ocaml/odoc#1116, @panglesd) - Fix wrong links to standalone comments in search results (ocaml/odoc#1118, @panglesd) - Remove duplicated or unwanted comments with inline includes (@Julow, ocaml/odoc#1133) - Fix bug where source rendering would cause odoc to fail completely if it encounters invalid syntax (@jonludlam ocaml/odoc#1208) - Add missing parentheses in 'val (let*) : ...' (@Julow, ocaml/odoc#1268) - Fix syntax highlighting not working for very large files (@jonludlam, @Julow, ocaml/odoc#1277)

Remove tyxml from odoc_html_frontend

e09933c

Also include a bugfix : name with no prefix are not displayed with a `.` in front.

changes

29e633b

panglesd approved these changes Jan 17, 2024

View reviewed changes

format

0a8ff5d

EmileTrotignon mentioned this pull request Feb 6, 2024

Do not duplicate code from odoc art-w/sherlodoc#19

Open

panglesd reviewed Feb 6, 2024

View reviewed changes

CHANGES.md Outdated Show resolved Hide resolved

EmileTrotignon and others added 2 commits February 12, 2024 13:53

prefix name now includes the dot

a560d45

Update CHANGES.md

eb2c7c5

panglesd merged commit b7a7d67 into ocaml:master Mar 18, 2024
8 of 10 checks passed

jonludlam mentioned this pull request Jan 23, 2025

[new release] odoc (5 packages) (3.0.0~beta1) ocaml/opam-repository#27309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove tyxml from odoc_html_frontend #1072

Remove tyxml from odoc_html_frontend #1072

EmileTrotignon commented Jan 17, 2024

panglesd commented Jan 17, 2024

trefis commented Jan 17, 2024 •

edited

Loading

EmileTrotignon commented Jan 17, 2024

EmileTrotignon commented Jan 17, 2024

panglesd commented Jan 17, 2024

EmileTrotignon commented Jan 17, 2024

EmileTrotignon commented Jan 17, 2024 •

edited

Loading

Drup commented Jan 17, 2024

panglesd left a comment

panglesd Jan 17, 2024

EmileTrotignon Jan 17, 2024

EmileTrotignon Jan 17, 2024

panglesd Jan 17, 2024

EmileTrotignon Jan 18, 2024 •

edited

Loading

EmileTrotignon Jan 18, 2024

EmileTrotignon commented Jan 17, 2024

Remove tyxml from odoc_html_frontend #1072

Remove tyxml from odoc_html_frontend #1072

Conversation

EmileTrotignon commented Jan 17, 2024

panglesd commented Jan 17, 2024

trefis commented Jan 17, 2024 • edited Loading

EmileTrotignon commented Jan 17, 2024

EmileTrotignon commented Jan 17, 2024

panglesd commented Jan 17, 2024

EmileTrotignon commented Jan 17, 2024

EmileTrotignon commented Jan 17, 2024 • edited Loading

Drup commented Jan 17, 2024

panglesd left a comment

Choose a reason for hiding this comment

panglesd Jan 17, 2024

Choose a reason for hiding this comment

EmileTrotignon Jan 17, 2024

Choose a reason for hiding this comment

EmileTrotignon Jan 17, 2024

Choose a reason for hiding this comment

panglesd Jan 17, 2024

Choose a reason for hiding this comment

EmileTrotignon Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

EmileTrotignon Jan 18, 2024

Choose a reason for hiding this comment

EmileTrotignon commented Jan 17, 2024

trefis commented Jan 17, 2024 •

edited

Loading

EmileTrotignon commented Jan 17, 2024 •

edited

Loading

EmileTrotignon Jan 18, 2024 •

edited

Loading