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

Global Policies #126

Open
vincentjames501 opened this issue Jun 23, 2016 · 12 comments
Open

Global Policies #126

vincentjames501 opened this issue Jun 23, 2016 · 12 comments

Comments

@vincentjames501
Copy link

Our company is using Selmer to format user-provided strings in our system. With that said, it would be nice to be able to do some of the things below. Some of these may go against Selmer philosophy, but it's worth a discussion. This may help Selmer gain adoption in more applications than the normal server-sider rendering.

Disabling Tags

It would be nice if there was some mechanism to disable tag parsing all together. Our company really only wants to do variables and filters. It would be nice to get this result:

(render "Hello {{name}}, {% blah %} how are you?" {:name "John"}) 
=> "Hello John, {% blah %} how are you?"

We can do this by setting tag-second to something like #"" but it would be clearer/cleaner if it was a global policy similar to (validate-off!)

Disabling Filters

While our company doesn't have this requirement, it may make sense to add this in relation to the above.

Missing Values

Right now this is the current behavior:

(render "Hello {{name}}, how are you?" {}) 
=> "Hello , how are you?"

Our company would prefer this say something like Hello {{name}}, how are you? or Hello <name not provided>, how are you? instead. What if we implemented something like this when a lookup fails in the context-map?

(set-missing-value-formatter! 
  (fn [arg context-map]
    (comment "We could do what ever we want with the arg here, maybe even reference another value in the context-map, throw an exception, etc.")
  (str "<" (name arg) " not provided>")))

And it could simply default to (constantly "")

Exceptions

As we're dealing with user provided strings, we would prefer (as we deal with emergencies) that no exceptions be thrown when there is a parse failure, but instead, simply return the failed tokens. What I mean by that is this:

(render "Hello {{name | this-filter-does-not-exist}}, how are you?" {:name "John"}) 
=> "Hello {{name|this-filter-does-not-exist}}, how are you?"
(render "Hello {% does-not-exist-tag %}, how are you?" {:name "John"}) 
=> "Hello {% does-not-exist-tag %}, how are you?""

Right now we have a solution for all of the above by manually looping though the selmer.node.INodes and manually calling .render-node but it would be neat to have some of these configurable as global policies.

Thoughts?

@yogthos
Copy link
Owner

yogthos commented Jun 23, 2016

I like all of the suggestions. Thanks for the PR for disabling tags/filters. For the missing values, I like the idea of the opt-in formatter, this way it won't affect existing users and provides flexibility for different users. Likewise, I would suggest the ability to specify the default error handler similarly to the missing value handler.

If you'd like to do a pr for those two, I can push out a release with the changes.

@yogthos
Copy link
Owner

yogthos commented Jun 26, 2016

And just as FYI, I pushed out 1.0.7, with the first two changes.

@casperc
Copy link
Contributor

casperc commented Sep 19, 2016

I would just like to chime in with the missing value behaviour - for our use case it is both needed to be able to provide a default value for missing values and throw an error in different situations.

It would also be useful to have a 'validate-render' to get an explicit exception when there are values being referenced which are not present.

@yogthos
Copy link
Owner

yogthos commented Sep 19, 2016

I definitely think this is a good idea. Unfortunately, I probably won't have time to dig into this in the near future. If anybody has time to do a PR for this however, I'd be glad to help out with that.

@casperc
Copy link
Contributor

casperc commented Sep 20, 2016

I might try to do a PR for this. I was looking through the code yesterday, and I could use a pointer for how best to determine that a value is missing.

It looks like the FunctionNode method .render-node (and handler function) always returns an empty string if the value is missing. If it where to return a nil on missing values, that could be used as a signal that the value was missing.

The default missing value formatter should then just treat nils and empty values the same, but a different user-implementation could e.g. throw an exception.

Thoughts?

@vincentjames501: you mention that you handle the cases you list by calling .render-node on INodes directly. Do you also handle the missing value case this way, and how to you do so?

@yogthos
Copy link
Owner

yogthos commented Sep 20, 2016

Yeah I think that would be a reasonable way to do things.

I'm thinking another approach to the problem could be to just use Schema or Spec to validate and coerce the input map before it even gets to the parser.

@casperc
Copy link
Contributor

casperc commented Sep 21, 2016

I added a pull request for the missing value functionality here: #130

I took the approach to make .render-node on FunctionNodes return nil which makes the missing-value-formatter fn get called with the metadata on the handler fn in the FunctionNode and the context map.

I am not quite sure what to do about the behaviour with regards to filters when there is a missing value. I have updated my original commit which had the value not be passed through the filters when it was nil, but this breaks some tests that are e.g. doing a count on a missing value and returning 0 (:count) or an empty string (in the case of e.g. upper).

For me it would be preferable to not pass the value to filters, which in a lot of cases return an empty string, even though the user has set a missing value handler and is expecting that to be triggered.

An option is to also set a switch when the user sets a custom missing value handler, which bypasses the filters. Could be bypass-filters-on-missing-value or something similar. That would maintain the current default behaviour, but allow a user setting a custom missing-value-handler to get the return value that instead of whatever the filters happen to return.

@yogthos
Copy link
Owner

yogthos commented Sep 21, 2016

Yeah, I agree that if the value is nil, there's no sense passing it to the filters at that point. Now that there's a way to set the missing value handler, you would supply a custom one if you wanted to have a value and run filters on it.

@casperc
Copy link
Contributor

casperc commented Sep 21, 2016

You beat me to it, but I have some additions which preserves the current functionality when no custom missing value handler is provided.

Then (= "0" (render "{{missing|count}}" {})) when not setting a custom handler is provided, but e.g. (= "<missing value: name|count>" (render "{{name|count}}" {})) when a custom handler is provided. I made it as a separate switch, so you can chose the behaviour you need, but with sensible defaults.

I'll push it in a few minutes, if I can figure out how now that the pull request is merged :)

@yogthos
Copy link
Owner

yogthos commented Sep 21, 2016

Oh yeah might have to be a new pr then, preserving the existing behavior would be nice though as it's less disruptive to anybody currently using it come to think of it. :)

@casperc
Copy link
Contributor

casperc commented Sep 21, 2016

Added a new PR: #131

The 1.0.8 version is still not breaking changes, just less useful than this, so version should probably be bumped again with a new release.

I have updated the docs with an explanation as well.

@yogthos
Copy link
Owner

yogthos commented Sep 21, 2016

Fantastic, I'll push out a new release with the improvements. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants