JustusAdam / mustache

Haskell implementation of mustache templates
BSD 3-Clause "New" or "Revised" License
99 stars 31 forks source link

Making lambdas more like the mustache spec #49

Open brandonchinn178 opened 3 years ago

brandonchinn178 commented 3 years ago

The mustache spec gives this example:

Template:

{{#wrapped}}
  {{name}} is awesome.
{{/wrapped}}

Hash:

{
  "name": "Willy",
  "wrapped": function() {
    return function(text, render) {
      return "<b>" + render(text) + "</b>"
    }
  }
}
Output:

<b>Willy is awesome.</b>

Issue https://github.com/JustusAdam/mustache/issues/30 talked about making lambdas easy to use for simple text transformations, but it's still pretty difficult to do anything more complicated. I'm thinking it would be nice if the Haskell API could align more with the API in the mustache spec, something like

instance ToMustache (Text -> (Text -> SubM Text) -> SubM Text)

The mustache example could then be implemented as

object
  [ "wrapped" ~> \text render -> do
      result <- render text
      return $ "<b>" <> result <> "</b>"
  ]

where the first argument is the raw template text.

Thoughts?

brandonchinn178 commented 3 years ago

As an example, I'm doing the following right now:

"withCondition" ~> \stree -> pure @SubM $
  case condition of
    Nothing -> stree
    Just cond -> concat
      [ [TextBlock $ "#if " <> cond <>"\n"]
      , stree
      , [TextBlock "#endif\n"]
      ]

and I'm thinking something like the following could be nice:

"withCondition" ~> \text render -> do
  case condition of
    Nothing -> render text
    Just cond -> do
      result <- render text
      return $ Text.unlines
        [ "#if " <> cond
        , result
        , "#endif"
        ]
JustusAdam commented 3 years ago

Do you mean this as "in addition" to the other allowed lambdas? In that case that seems like a reasonable addition, since it is also backwards compatible.

JustusAdam commented 3 years ago

If you would like to implement this, I'd be happy to accept a PR.

brandonchinn178 commented 3 years ago

I'd be happy to submit a PR, but I'm not quite sure what the best way forward is. The mustache spec requires that lambdas pass through the raw template text, but STree seems to throw away the raw template text? I think we'd need to change STree to

data STree a = STree [Node a] Text

but I'm not sure how to store the raw text at the same time as parsing it

brandonchinn178 commented 3 years ago

I started work in https://github.com/JustusAdam/mustache/pull/50. I also added the spec tests related to lambdas, and I'm running into two issues:

  1. What should I replace the TODO with in Text.Mustache.Parser?
  2. How do I account for alternate delimiters when rendering the lambda result?
JustusAdam commented 3 years ago
  1. I haven't read your code super closely, but I assume the issue is how to get the raw Text between two tags? (If this assumption is wrong please correct me) My suggestion would be when a section is opened, store the current location in the text in the sections stack (I'm sure parsec, the parsing library, provides some function to retrieve the current index into the underlying text) and then, when the section closes slice the input Text by this index and the index just before the closing section tag. That would be quite an efficient way to get the raw text and should require relatively little parser modification.

  2. Rendering the result is indeed a possible issue. However we need to reparse the text anyway if I understand it correctly? So you could do that with parseWithConfig, which allows you to pass in a config that can set a delimiter. This means that you'd have to store the currently set delimiter in the Lambda node as well and it would default to the default delimiters.

I have one further comment: I would also be in favor of an optimized version of this whole interface for functions of type (STree -> (Stree -> SubM Text) -> SubM Text) where we pre-parse the text in conjunction with an IsString and Semigroup instance for STree, which would allow similar code to what you want, but without requiring reparsing. This would then be accompanied by a comment explaining that the Text based lambda is inherently inefficient and should not be used if performance is of concern (i.e. a web-service) and also documentation for the IsString instance for STree (possibly repeated in the documentation for STree that this instance does not parse the string and just returns a TextNode

brandonchinn178 commented 3 years ago

So actually, overText is sufficient for my use case, so I have no pressing need to implement this. And for most use cases, overText is probably sufficient.

However, it's not fully compliant with the mustache spec for lambdas — in fact, none of the current instances fully implement the mustache spec. For example, the mustache spec states that the lambda must be able to inspect the raw template text, and lambdas must be able to return a template that would be rendered. Now, maybe that's fine; since lambdas are optional, we could say that the mustache library does not implement the official lambda spec, but rather it implements some mustache_haskell_lambda spec.

I don't really have skin in the game, so I'll leave whatever next steps up to you. I did update my PR to add tests for the official lambda spec, which also includes a couple other library changes. Feel free to cherry-pick and/or close the PR, for whatever you decide