Specify external renderer via module and id (related to mbutterick/pollen-users#94) #253

Changes how external renderers are specified:

(module setup racket/base
  (provide (all-defined-out))
  (define external-renderer '(beeswax/for-pollen render))

This avoids any loading loops with pollen/setup.

I can add documentation before merging too if you want!

Changes how external renderers are specified: ```racket (module setup racket/base (provide (all-defined-out)) (define external-renderer '(beeswax/for-pollen render)) ``` This avoids any loading loops with `pollen/setup`. I can add documentation before merging too if you want!

Why is this patch necessary? This is the only way to fix the loading loop?
What are the performance implications? (The new match code will be run every time any file is rendered.)

1. Why is this patch necessary? This is the only way to fix the loading loop? 2. What are the performance implications? (The new `match` code will be run *every time* any file is rendered.)

Why is this patch necessary? This is the only way to fix the loading loop?

No, not necessary strictly speaking. It also works to never require any Pollen modules anywhere in the vicinity of the external renderer function. But avoiding it is tricky and cumbersome.

I’m sure there's another way that hasn’t occurred to me. Do you see downsides to this approach?

What are the performance implications? (The new match code will be run every time any file is rendered.)

It’s a fair point; I thought of making it a caching procedure but wanted your thoughts on the approach first. Even simpler, maybe it should just not be a one-arity function that gets called every time? E.g.:

(define external-renderer
  (match (setup:external-renderer)
    ...

>1. Why is this patch necessary? This is the only way to fix the loading loop? No, not necessary strictly speaking. It also works to never `require` any Pollen modules anywhere in the vicinity of the external renderer function. But avoiding it is tricky and cumbersome. I’m sure there's another way that hasn’t occurred to me. Do you see downsides to this approach? >2. What are the performance implications? (The new `match` code will be run _every time_ any file is rendered.) It’s a fair point; I thought of making it a caching procedure but wanted your thoughts on the approach first. Even simpler, maybe it should just not be a one-arity function that gets called every time? E.g.: ```racket (define external-renderer (match (setup:external-renderer) ... ```

Doing it that last way would actually make it faster than the current version, since it would no longer call (setup:external-renderer) every time a file is rendered.

Doing it that last way would actually make it faster than the current version, since it would no longer call `(setup:external-renderer)` every time a file is rendered.

I think it would suffice to move the #false test out of your external-renderer function. So instead of this:

(λ () ((or (setup:external-renderer) render)))

We could have this:

(λ () ((or (let ([val (setup:external-renderer)])
             (and val (external-renderer val))) render)))

IOW, the parsing of the value (by calling your new external-renderer function) would only happen if (setup:external-renderer) is not #false. So the costs of the new function would be borne only by those using an external renderer.

I think it would suffice to move the `#false` test out of your `external-renderer` function. So instead of this: ``` (λ () ((or (setup:external-renderer) render))) ``` We could have this: ``` (λ () ((or (let ([val (setup:external-renderer)]) (and val (external-renderer val))) render))) ``` IOW, the parsing of the value (by calling your new `external-renderer` function) would only happen if `(setup:external-renderer)` is not `#false`. So the costs of the new function would be borne only by those using an external renderer.

I'll amend the PR as soon as I have time. Worth adding to the docs yet or hold off?

Let’s leave it out of the docs for now, as you may end up having other ideas as you refine Beeswax.

PS

Even simpler, maybe it should just not be a one-arity function that gets called every time? E.g.:
(define external-renderer
  (match (setup:external-renderer)
    ...

I don’t think this would work because the result of any call to a setup value is potentially different for every file being rendered. (Extreme case: every source is in a different directory with its own pollen.rkt). With this approach, the setup value is only evaluated once.

PS > Even simpler, maybe it should just not be a one-arity function that gets called every time? E.g.: > > ```racket > (define external-renderer > (match (setup:external-renderer) > ... > ``` I don’t think this would work because the result of any call to a `setup` value is potentially different for every file being rendered. (Extreme case: every source is in a different directory with its own `pollen.rkt`). With this approach, the `setup` value is only evaluated once.

Thanks! I tweaked this patch to rely on the default exception messages, because they have other useful information.

What have you discovered about this hook for external rendering? Does it work well enough that we should commit it to the docs? Or do you want to have more time to experiment?

It’s working great the way it is, and no further changes or ideas have occurred to me. I’d vote for adding documentation at this point.

Labels Milestones

Specify external renderer via module and id (related to mbutterick/pollen-users#94) #253

Step 1:

Step 2: