edwindj / whisker

{{mustache}} for R
https://mustache.github.io
213 stars 19 forks source link

Support R variables with . and data.frames with $ #11

Closed KentonWhite closed 10 years ago

KentonWhite commented 10 years ago

I was incorporating whisker into ProjectTemplate at a user request and was having problems handling variables that have '.' in their name as well as referencing columns of a data.frame using the '$' notation.

Sure enough, in the code there was a TODO about this!

The code looks like it is splitting variables on '.' to create nested object method calls. E.g if I pass in "foo.bar" the variable name is foo and the method name to call on foo is bar. To the best of my knowledge, method calls on objects in R don't use '.'.

The common use cases that I'm aware of are using '$' to reference a column of data on a data.frame and using '@' to access a slot on an object. I've updated the code to handle variable names with '.' and to handle the '$' on data.frames. E.g. if I pass in "foo$bar" it will take the variable name to be foo and the column to return as bar. If I pass "foo.bar" it will take the variable name to be foo.bar.

I didn't make changes for the slots, but made a TODO note.

edwindj commented 10 years ago

Thanks for your addition! But it is a difficult one:

whisker conforms to the mustache language: it is programming language agnostic. Changing "." into "$" lets whisker fail many compliance tests:

library(devtools)
test_package("whisker")

I suggest that we implement it as an option: the default is mustache compliant, but you can specifiy, via options, or whisker.render(..., use_dollar=TRUE)

Would that be an option?

Best

KentonWhite commented 10 years ago

Ah yes! I see now on the mustache spec for interpolation the using '.' as the context sigil is written into the spec. In this (discussion thread)[https://github.com/mustache/spec/issues/52] there is mention of the difficulty of writing about the context stack in an implementation agnostic way. The gist is that context stack implementation should be left to the implementors.

I'll ask at mustache/spec for clarification.

In the mean time, why not add whisker.render(..., context_sigil = '$'). This can default to the '.' context sigil but also leaves it open for people wanting to access object slots with a '@' context sigil.

KentonWhite commented 10 years ago

Got some feedback from other Mustache implementors. Specifically he says

However, the spec does not enforce that a single input (data + template) should render the same in all implementations. The weak spot is the way booleans are handled (see issue #4). In some languages, 0 (zero) would be truthy, and in other languages, the same value would be falsey.

So whisker should free to define interpolation to be the natural usage for R. In fact, other Mustache implementors encourage this!

Looking at test interpolation.R, the tests for context interpolation represent the data structure person = list(name = "Joe") as person.joe. In my opinion, this is not intuitive. If I have person = list(name = "Joe") loaded in my environment, person.name gives Error: object 'person.name' not found. This requires me to know a "hidden" syntax to make my Mustache templates work in whisker.

I've played around with the tests, and using '$' instead of '.' works except for the method chaining part of the spec since R doesn't allow for chaining of data frames.

Maybe instead of having options context_sigil or user_dollar there can be an option strict. Using strict will interpret the template strictly according to the spec. Otherwise the template will be interpretted using a more natural R syntax. The default can be FALSE and set to TRUE for testing against the spec. Out of the box whisker would support interpolation along the lines of what I think users would expect. And for those really concerned about reusing templates from other languages, and investing the time to make R data that complies, they can set strict = TRUE.

groue commented 10 years ago

To clarify: I'm not a Mustache maintainer. I'm a template engine implementor, like you.

KentonWhite commented 10 years ago

@groue sorry for the misunderstanding. I've updated to comment to avoid any further confusion.

groue commented 10 years ago

@KentonWhite You're welcome. Actually Mustache is no longer maintained: the people who are responsible of the specification repository haven't posted any answer to any comment in at least a year now.

ctbrown commented 10 years ago

Interesting thread. I think that this is a problem with R that I have has some grumblings. The allowed use of . in name was a rather poor decision by the R/S development team when so many other languages were using it as a (dereference) operator.

edwindj commented 10 years ago

I'm happy to add $ support, but the default (for the moment) will be the .. This is because it is a breaking change and some packages depend on it. (e.g. rMaps and rChart use whisker to generate html and javascript).

KentonWhite commented 10 years ago

Thanks! Closing since changes merged into master.