search

ocaml / ood

OCaml.org v3 data repository
Other
13 stars 8 forks source link

Migrate v3 #44

Closed tmattio closed 3 years ago

tmattio commented 3 years ago

Counterpart of v3's PR that implements various fixes required to migrate v3 to Ood's next branch:

tmattio commented 3 years ago

Ok, this is ready I think.

The last commit adds the table of content for the tutorials. It works by leveraging the toc functions in Omd's PR https://github.com/ocaml/omd/pull/240.

I vendored omd to to include the patches in the PR, it can be unvendored once the functions are upstreamed.

Omd's HTML generator will add Omd attributes as HTML attributes, and the toc functions will create links to the "id" attribute if it's present, so the only thing needed to implement a working TOC is to add the IDs to the document before creating the TOC and generating the HTML.

Now, I tested several approaches and initially wanted to include a structure for the TOC, but after considering it, I'm not finding any convincing argument that justify the added complexity in both ood and v3 (to generate the TOC from this structure), when compared to injecting the HTML directly, as we do with the tutorial's content. Granted, the flexibility will be appreciated if we do some complex operations with the TOC, but I don't think this is the case at the moment, and the CSS can be applied to a root element to style the TOC.

@agarwal any thought on this last point?

I'll wait for the V3 counterpart PR to be merged before merging this

tmattio commented 3 years ago

I don't think there anything to change here, so merging now 🙂

ghost commented 3 years ago

. Granted, the flexibility will be appreciated if we do some complex operations with the TOC, but I don't think this is the case at the moment, and the CSS can be applied to a root element to style the TOC.

The main benefit that I see is allowing new contributors to v3 to restructure the html as well as the CSS, instead of only being able to adjust the CSS, without having to dive into the ood transformations.

tmattio commented 3 years ago

The main benefit that I see is allowing new contributors to v3 to restructure the html as well as the CSS, instead of only being able to adjust the CSS, without having to dive into the ood transformations.

I agree, that's also what made me hesitant. On the other hand, to play devil's advocate, I don't think the table of content will change much once we're happy with the design, and the HTML for it is fairly standard, something else would seem dubious to me, unless we want to add more elements, such as icons, etc., but then we'd need changes in ood anyways for these.

So while I agree this may be useful, I'm still thinking we can wait for a clear use case to become apparent before implementing this, what do you think?

ghost commented 3 years ago

So while I agree this may be useful, I'm still thinking we can wait for a clear use case to become apparent before implementing this, what do you think?

At this point, you can see the full landscape of options, so it's really up to you. I like maximizing the html rendering in v3, but I understand your point about how stable this portion might be.

Personally, I would probably leave open a draft PR to implement the other approach and then consider polishing and merging the PR if the need arises later, or close the PR without merging if the need doesn't arise after a few months.