URN expressions #145
URN expressions #145
Conversation
| @@ -317,6 +318,10 @@ func EvaluateTemplateAsString(env utils.Environment, resolver utils.VariableReso | |||
| buf.WriteString(token) | |||
| } else { | |||
| strValue, _ := utils.ToString(env, value) | |||
| if urlEncode { | |||
There was a problem hiding this comment.
same as in old engine - need this for when the expressions should be URL encoded, but the template itself shouldn't
| nesting string | ||
| nestedVars vars | ||
| type varMapper struct { | ||
| substitutions map[string]string |
There was a problem hiding this comment.
these are things which get totally rewritten - e.g. becomes a function call. Also renamed other fields for clarity.
| } | ||
|
|
||
| var rootVarMapper = newRootVarMapper() |
There was a problem hiding this comment.
avoids re-initializing this for every expression
| @@ -247,15 +271,15 @@ func migrateLegacyTemplateAsString(resolver utils.VariableResolver, template str | |||
| } else { | |||
| strValue, _ := toString(value) | |||
|
|
|||
| // date context references get rewritten as functions | |||
There was a problem hiding this comment.
so do other things now - hence wrapRawExpression
|
@nicpottier please re-review - format_urn now accepts a list |
excellent/functions.go
Outdated
| sliceLen, _ := utils.SliceLength(urnArg) | ||
| if sliceLen >= 1 { | ||
| urnArg, _ = utils.LookupIndex(urnArg, 0) | ||
| } |
There was a problem hiding this comment.
Maybe 0 case should be explicitly handled here? Either as an error or an empty string? (I lean towards empty string though could be easily swayed to it being an error)
There was a problem hiding this comment.
I prefer empty string so that @contact.urns and @(format_urn(contact.urns)) both evaluate to empty (when stringified)
excellent/functions.go
Outdated
| // | ||
| // @(format_urn("tel:+250781234567")) -> 0781 234 567 | ||
| // @(format_urn("twitter:134252511151#billy_bob")) -> billy_bob | ||
| // @(format_urn("NOT URN")) -> ERROR |
There was a problem hiding this comment.
should we add a @(format_urn(contact.urns)) example?
excellent/legacy_test.go
Outdated
| {old: "@contact.telegram", new: "@(format_urn(contact.urns.telegram.0))"}, | ||
| {old: "@contact.twitter", new: "@(format_urn(contact.urns.twitter.0))"}, | ||
| {old: "@contact.facebook", new: "@(format_urn(contact.urns.facebook.0))"}, | ||
| {old: "@contact.mailto", new: "@(format_urn(contact.urns.mailto.0))"}, |
There was a problem hiding this comment.
Do we need the .0s here?
There was a problem hiding this comment.
No but I think better the users see that these are lists and we treat passing a list to format_urn as a convenience.
There was a problem hiding this comment.
Hah, funny, feels the opposite to me. Since 99% of users will never have more than one URN, present them with the simplest option that works.
There was a problem hiding this comment.
Well I guess I'd be ok with making this look simpler. I actually wonder tho how common format_urn usage is going to be since I imagine most of the time people want to reference contact URNs, it's not to display them in a message, but to pass them to a webhook or something like that
See #63