jamboree / bustache

C++20 implementation of {{ mustache }}
82 stars 10 forks source link

Make context misses trigger an error #5

Closed sjoubert closed 6 years ago

sjoubert commented 6 years ago

As per mustache(5), it is suggested that variable misses default to empty value, but also that this behavior be configurable in the library. This is not the case for bustache.

I'm willing to contribute such an improvement if you consider it acceptable. However, I have a few questions:

  1. The spec does not suggest a similar behavior for section misses. However, I'd really like to trigger an error too. My use case for misses triggering errors is that I don't want my template file to contain unused elements and that any typo between the format and the context be spotted as early as possible. Do we handle variable and section misses the same or do we use two different configuration switches?
  2. I think the best way to configure the behavior is to pass an option at generate time, in the same way the html_escape behavior is provided. I think the best way would be to convert option_type to a proper option struct with multiple configuration switches. Wha't your policy regarding breaking changes? Is there another way to do that?

Thanks.

jamboree commented 6 years ago

I think the best way is to accept a callback that allows you handle the unresolved key yourself, which is more flexible than an option flag.

What do you plan to do with inverted section?

sjoubert commented 6 years ago

Indeed, a callback is more flexible, I'll see what I can do.

For the inverted section, I plan to also trigger the error/callback if the inverted section name is missing. The idea is really to have no room for typo being sneaked in the template/data. Basically, having the absence of something (variable, section or partial) should trigger an error/callback and not default to an empty/false value. So, one would have to explicitly set an empty list or a false value to trigger an inverted section.

What about the API? Should I add an additional parameter after option_type flag or refactor the option_type parameter?

jamboree commented 6 years ago

For the inverted section, I plan to also trigger the error/callback if the inverted section name is missing

Hmm, I think this is a common idiom in mustache:

{{#item}}{{field}}{{/item}}
{{^item}}No item to show.{{/item}}

Where the second line is the fallback if item is absent. But yes, using custom callback should cover this case as well.

What about the API? Should I add an additional parameter after option_type flag

Yup, just add another parameter, e.g.

struct default_unresolved_handler
{
    value operator()(std::string const& /*key*/) const
    {
        return nullptr;
    }
};

template<class Sink, class UnresolvedHandler = default_unresolved_handler>
inline void generate
(
    Sink& sink, format const& fmt, value::view const& data,
    option_type flag = normal, UnresolvedHandler&& f = {}
);

Another option is adding such a callback in the "model" instead, e.g.

object data
{
    {"", // A special unresolved callback
        [](std::string const& key)
        {
            // Custom behavior
        }
    },
    ...
};

But I'm not sure whether this is a good idea.

sjoubert commented 6 years ago

Thanks a lot for the work!

I started to work on it but did not have the time to finish it. Mostly because I didn't know how to handle name misses on partials, which this patch does not handle right?

The issue with partials is that you can't return a value, to be consistent one should return format. I tried to require UnresolvedHandler to have 3 overloads that would take unresolved ast::variable, ast::section and ast::partial then return value and format accordingly. But this seems way to complicated and exposes ast to the user. I don't know if you'll see an easy way to do that. I guess one solution would be to use a custom Context container and do what I want in a custom find method.

Another thing: this feature is not exposed by the format class. Does this means I need to use the generate_* function instead?

Anyway, thanks again for this.

jamboree commented 6 years ago

I didn't know how to handle name misses on partials, which this patch does not handle right?

Partials are treated differently than variables/sections, actually bustache already allows custom behavior on lookup. Let's see...

I guess one solution would be to use a custom Context container and do what I want in a custom find method.

Exactly. The context can be any type that satisfied this requirement:

So, if you want to wrap an existing context and make it throw on unresolved key, you can do this:

template<class Context>
struct strict_context_wrapper
{
    using value_type = typename Context::value_type;
    using iterator = typename Context::const_iterator;

    iterator find(std::string const& key) const
    {
        auto it = ctx.find(key);
        if (it == ctx.end())
            throw std::runtime_error("unresolved partial: " + key);
        return it;
    }

    iterator end() const
    {
        return ctx.end();
    }

    Context& ctx;
};

template<class Context>
inline strict_context_wrapper<Context> make_strict_context(Context& ctx)
{
    return {ctx};
}

See full example here.

this feature is not exposed by the format class. Does this means I need to use the generate_* function instead?

Yes. Bustache is designed to be modular, that means, if one doesn't like the object model and generator, he can just use the format part of this lib and use his own object and generating function (though it'll be a huge effort). As you can see, the UnresolvedHandler is coupled with the object model, but format doesn't depend on the object model by design, so you have to use the generate_* function directly or write some helper functions to ease the use.