Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP feat: add monadic support #74

Closed
wants to merge 1 commit into from

Conversation

michivi
Copy link

@michivi michivi commented Dec 8, 2020

This PR would add support for monadic transformation and rendering.

The main motivation is to give the possibility for transforming and rendering in a certain monad (as requested in #46 ). For example, one might want to call an external service to render a specific content.

The scan alternative (i.e. doing a scanning beforehand, then send the results to the rendering extension) is not enough as scanning only consider the original content, not the one that might get constructed from the content and the application of other extensions. Adding the possibility to scan over the transformed content would solve that problem, but leave something to be desired (intermediate information generated, having some internals hidden but the transformed content publicly visible...).

This PR would solve the problem by introducing monad support for MMark, support that is already present in Lucid.

While the code should not introduce major breaking changes, there's still some open question regarding performance and ease of use (MMarkM m might seem a little weird, just as the parse function that just doesn't care about the monad).

Is this PR worth investigating?

@mrkkrp
Copy link
Member

mrkkrp commented Dec 14, 2020

Thanks! I did not forget about this PR. I'll review it when I have time, probably this weekend.

@michivi
Copy link
Author

michivi commented Dec 16, 2020

No worries. :-)

My thoughts on the PR:
Though I can successfully use the monad for side-effects, it seems weird to have to mention a monad at all when using MMark (well, the newly introduced MMarkM m actually). As it is, extensions are only used for rendering.

Perhaps it would be more interesting to split MMark into two separate data structures: one for the actual blocks and YAML data, and another one for the extensions. The usage of the PR would be cleaner as no type parameter would be required for the former, but some effort will go into backward compatibility. The extensions would only be required when rendering. Scanning can use a monad which can be completely different from the one given in MMarkM.

At least, I believe it would simplify my usage of the library :-)

@mrkkrp
Copy link
Member

mrkkrp commented Jan 11, 2021

I looked at this today. To be frank I'm reluctant to follow this path because it'd result in duplication of almost the entire API.

For example, one might want to call an external service to render a specific content.

But this is still possible even with the current code, right? You just need to perform all your effects before you start rendering. This way you can prepare all necessary information (e.g. fetch something via HTTP) and then use via the extension mechanism. The only use I can see for introducing a monad is for manipulation of a state during rendering, e.g. if you want to assign an integer per link.

The scan alternative (i.e. doing a scanning beforehand, then send the results to the rendering extension) is not enough as scanning only consider the original content, not the one that might get constructed from the content and the application of other extensions.

It looks like the trans/render extension-constructors could allow the user to inspect inlines constructed so far (right now we only show the original inlines inside the Ois wrapper). If this is indeed what is desired, it would be a more elegant enhancement, more in the spirit of the library. I think what I tried to do is to avoid tangling and interweaving of extensions. The way it is done is that you have the HTML rendition constructed so far and you can add something around it (or just before/after), but you only can inspect the original inline to decide what to do. If it weren't the case extensions could start interacting in a confusing and hard-to-debug way. For example, you could have an extension that transforms links. Then you could have another extension that creates a navigation form. You may or may not want to transform the navigation links in the same way you transform links that come from the original document. Right now the behavior is such that the navigation links will essentially be out of reach for other extensions and the code that adds them should decide their final appearance and properties. Granted, I see how this can turn out to be limiting for certain applications, but I think that the idea is sane.

@michivi
Copy link
Author

michivi commented Jan 12, 2021

I agree with your vision for simplicity, and that is completely in line with the package philosophy.

Just for completeness, here was my goal. For my (very specific) use case of rendering some specific blocks using external services, the usage just seemed weird.

  1. First translate some blocks into other blocks (translate the content into the dialect of external service B using service A)
  2. Then transform those translated blocks using the external service B

I can't just use the MMark API as is, as I need some side-effects.
Thus, the steps I thought I would have to follow with scanning would then be:

  1. Parse the markdown document
  2. Scan the document for those specific blocks
  3. Generate a mapping for those blocks using extension and service A
  4. Add extension A with those mappings
  5. Scan the document for the mapped blocks (<-- won't actually find any as scan only sees the original blocks)
  6. Generate a mapping of those blocks into the final blocks using extension and service B
  7. Add extension B with those final mappings
  8. Purely render to HTML

This solution doesn't work as scanning only sees the original blocks. And I wanted to keep extensions A and B separate for modularity and reusability.

One of the points of this PR was to know if there was another way, preserving composition and reasonability. Monads would allow for a single pass, but agreed, in exchange for complexity and heavy API modifications. I concur that the PR may not be the best idea :-) But I don't know if bad interactions are possible this way, so long as they only see the currently transformed structure (just as with function composition?). Extensions would still be testable independently. And in some cases (such as here), interactions might just be what we are looking for. In all cases, I believe the user may be able to decide using the order of the extensions?

I also have to admit that this is clearly not the everyday use case. For those type of cases and to keep the API simple, wouldn't it be possible to keep the existing API as is (not introducing a constructor to access the tranformed content), but have and expose an intermediate data structure for MMark?

The process could be:

Input --> MM Parsing --> Intermediate MD --> Custom block rendering --> Intermediate MD --> MM Processing --> HTML
                               ^                      ^                                           ^
                               |                      |                                           |
                    Parsed and transformable          |                              Existing MMark processing
                           Markdown                   |
                                              External service

The upside is that the client is free to do whatever it wants with the structure before rendering, including side-effects. Or it can also use the existing API, preserving performance.
The downsides are that some internal structure would be exposed, and the line with this and extensions would be kind of blurry...

@mrkkrp
Copy link
Member

mrkkrp commented Jul 8, 2024

I'm going to close this since there hasn't been much activity on this PR for a while. I've just merged #116 which hopefully will make implementing custom renders posible.

@mrkkrp mrkkrp closed this Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants