-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
Comments
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. |
And just as FYI, I pushed out |
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. |
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. |
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? |
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. |
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. |
Yeah, I agree that if the value is |
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 :) |
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. :) |
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. |
Fantastic, I'll push out a new release with the improvements. :) |
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:
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:
Our company would prefer this say something like
Hello {{name}}, how are you?
orHello <name not provided>, how are you?
instead. What if we implemented something like this when a lookup fails in thecontext-map
?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:
Right now we have a solution for all of the above by manually looping though the
selmer.node.INode
s and manually calling.render-node
but it would be neat to have some of these configurable as global policies.Thoughts?
The text was updated successfully, but these errors were encountered: