search

typelevel / Laika

Site and E-book Generator and Customizable Text Markup Transformer for sbt, Scala and Scala.js
https://typelevel.org/Laika/
Apache License 2.0
406 stars 44 forks source link

sbt plugin & preview server: allow to add custom renderers #588

Closed jenshalm closed 3 months ago

jenshalm commented 4 months ago

Introduces a new laikaRenderers setting for Laika's sbt plugin that allows to configure additional renderers. Previously, using your own or 3rd party renderers that don't come out of the box with Laika was trivial for API users, but very cumbersome for plugin users. This PR creates ease-of-use parity between these two user groups.

There are no new scripted tests as Laika's built-in renderers now use the new setting themselves, so it is implicitly tested by the existing suite.

An example for adding the search index generator from protosearch which was the initial trigger for introducing this new mechanic:

laikaRenderers += BinaryRendererConfig(
  alias = "index",
  format = IndexFormat,
  artifact = Artifact(
    basePath = Root / "search" / "searchIndex",
    suffix = "idx"
  ),
  includeInSite = someCustomSetting.value,
  supportsSeparations = false
)

With the configuration above, the search index can be generated by calling laikaGenerate index (or together with other formats in one transformation - e.g. laikaGenerate html index).

When the includeInSite property is true, the index will also be generated when calling laikaSite. This is ideally a decision left for the end user, which is what I indicated with someCustomSetting.value above.

@valencik / @samspills / @armanbilge: Since you are probably the first developers using this, it would be great if one of you could do a quick review for this. There is no urgency, this can be parked while I work on other little additions for 1.1.

The advantages of using it instead of your existing approach would be:

valencik commented 4 months ago

Thanks for the ping on this, hoping to test it out this week :)

jenshalm commented 3 months ago

@valencik did you have any chance yet to have a look?

The current status for 1.1 is that all planned PRs are open now, so it's now simply a matter of merging and cutting a release, which I'd love to do at some point in the coming 2 weeks.

Bugs can be fixed in a 1.1 patch if needed, so as long as you think the new API is sufficient we can merge this.

valencik commented 3 months ago

Hey @jenshalm, funny, I have an in progress review I started earlier this morning :)

I was going to test this by publishing locally, and then adding basically the snippet you've shown in the description to a project's build.sbt. Presumably that would work, and then someone could in theory use our custom TwoPhaseRenderFormat[Formatter, BinaryPostProcessor.Builder] without depending on the protosearch-sbt plugin specifically (just the Laika module). Does that sound right?

Additionally, I'm curious how this might simplify the protosearch-sbt plugin itself. Should I be able to use laikaRenderers from within the protosearch-sbt plugin? Perhaps simplifying some of Tasks.scala?

jenshalm commented 3 months ago

I was going to test this by publishing locally, and then adding basically the snippet you've shown in the description to a project's build.sbt. Presumably that would work, and then someone could in theory use our custom TwoPhaseRenderFormat[Formatter, BinaryPostProcessor.Builder] without depending on the protosearch-sbt plugin specifically (just the Laika module). Does that sound right?

Additionally, I'm curious how this might simplify the protosearch-sbt plugin itself. Should I be able to use laikaRenderers from within the protosearch-sbt plugin? Perhaps simplifying some of Tasks.scala?

Yes, I generally wonder whether you publishing an sbt plugin will be necessary in the future. Because the UI bits can also be bundled up in the lib module (as a theme extension) and then users can install both, the index renderer and the search UI with the two corresponding sbt settings. Preparing everything within the lib module would also mean users can as easily use it with other build tools like Mill. I can do the search UI bundling in a PR in protosearch if you want, it's quite straightforward.

valencik commented 3 months ago

I can do the search UI bundling in a PR in protosearch if you want, it's quite straightforward.

Oh my goodness, that would be ⭐ amazing 🌟 I would really appreciate that. Even something barebones to get started would be hugely helpful.

I generally wonder whether you publishing an sbt plugin will be necessary in the future.

Yeah perhaps protosearch could just publish a laika lib module, and then sbt-typelevel could do the plugin setup. I think @armanbilge has actually suggested as much before.

jenshalm commented 3 months ago

Yeah perhaps protosearch could just publish a laika lib module, and then sbt-typelevel could do the plugin setup. I think @armanbilge has actually suggested as much before.

Yes, that sounds like the simplest approach. sbt-tl depending on protosearch would probably be fine as both projects are in 0.x range.

jenshalm commented 3 months ago

Converted back to draft. It turns out that not only the sbt plugin itself needs to move from hardcoding EPUB and PDF output to having flexible renderer configuration, the preview server (which is not part of the plugin project) needs to make the same move.

This requires further changes to this PR:

jenshalm commented 3 months ago

@valencik this is back on track now, the preview server has been taught about custom binary renderers. This time I tested it myself with the protosearch integration and it's working when applied to the Laika manual (in both, laikaSite and laikaPreview).

If you want to save some time and test with a verified scenario first, here are the steps I took:

Changes to the laikaIO module in protosearch

First, I amended the open PR by copying the js generation in the build over to the laikaIO module so that the 4th file can actually be served from the jar.

Then I added a custom builder for creating the config for the renderer. I did this because I realized that most of the properties are not actually user-configurable. The user cannot choose the file path as the UI would stop working when things are not in the expected place. So I added a custom builder like this (which is used in one of the steps above):

  object IndexRendererConfig {

    def apply(includeInSite: Boolean): BinaryRendererConfig = {

      BinaryRendererConfig(
        alias = "index",
        format = IndexFormat,
        artifact = Artifact(
          basePath = Root / "search" / "searchIndex",
          suffix = "idx"
        ),
        includeInSite = includeInSite,
        supportsSeparations = false
      )
    }

  }

I did not add this to the PR since it's somewhat unrelated to the theme bundling, but I'd recommend you add this to the analysis package to minimize config boilerplate for users.

Using protosearch with the Laika sbt plugin is now down to 2 single-line statements which imho makes your sbt plugin redundant.

valencik commented 3 months ago

the preview server has been taught about custom binary renderers

This is excellent! I think that's going to make testing out search so much easier! I'll try and test out the process you've described this afternoon.

Just to double check,

In project/ManualSettings add .extendWith(SearchUI) below line 231.

I don't see a line 231 in either this branch or latest main https://github.com/typelevel/Laika/blob/plugin/render-config/project/ManualSettings.scala#L213

That IndexRendererConfig will be nice to have, there are perhaps a few configurable settings I could imagine in the future. But you're right that some are very much fixed.

jenshalm commented 3 months ago

Just to double check,

In project/ManualSettings add .extendWith(SearchUI) below line 231.

Oops, sorry, I had the IndexRendererConfig on the top of the file. I added it right before the last code line of the file (.build) which should be line 211 on the branch.

valencik commented 3 months ago

I've run through the above test you detailed (thank you!) and everything worked swell. I could browse to http://localhost:4242/latest/search/search.html and there was the search UI! 🎉 This will be a great usability improvement for testing things in protosearch. :)

A tiny note, in case it's interesting, I did have to run docs/mdoc before docs/laikaSite as I was getting the following error:

(docs / laikaSite) laika.io.internal.errors.ParserErrors: Multiple errors during parsing:
  Path does not exist or is not a directory:
    /home/andrew/src/github.com/typelevel/Laika/docs/target/mdoc

Would you be willing to release a milestone once this PR lands? Or does Laika publish snapshots I could use?

jenshalm commented 3 months ago

A tiny note, in case it's interesting, I did have to run docs/mdoc before docs/laikaSite as I was getting the following error:

Ah, yes, sorry, I forgot, this is expected when you don't use sbt-typelevel which provides the integration out of the box,

Would you be willing to release a milestone once this PR lands? Or does Laika publish snapshots I could use?

Everything merged to main should trigger a snapshot release.

Sorry, I somehow managed to edit your comment instead of quote-replying, but I reverted everything back.

jenshalm commented 3 months ago

@valencik snapshot is available here now: https://s01.oss.sonatype.org/content/repositories/snapshots/org/typelevel/laika-sbt_2.12_1.0/1.0.1-13-b6ca99f-SNAPSHOT/