#146 enhancement
Dan Dean

Template docs need mention of toTemplateReplacements feature

Reported by Dan Dean | February 14th, 2010 @ 02:59 AM

The documentation for both Template#evaluate and String#interpolate make no mention of the toTemplateReplacements feature. Add this to the Template section, at least.

I've found relevant examples here:

http://dev.rubyonrails.org/ticket/8166
http://www.prototypejs.org/2007/8/15/prototype-1-6-0-release-candidate

Comments and changes to this ticket

  • Dan Dean

    Dan Dean February 16th, 2010 @ 01:01 AM

    • Assigned user changed from “Samuel Lebeau” to “Dan Dean”
  • Dan Dean

    Dan Dean February 28th, 2010 @ 11:46 PM

    • Tag set to enhancement
    • State changed from “new” to “open”
  • Dan Dean

    Dan Dean March 1st, 2010 @ 12:12 AM

    • State changed from “open” to “enhancement”
    • Tag cleared.
  • Dan Dean

    Dan Dean March 4th, 2010 @ 04:48 AM

    • Tag set to needs:review

    I've updated the docs for the Template class and String#interpolate method. My goal with this is to provide easy to understand examples to all of the various template uses, which have so far been under-documented. I also tried to make the introduction more concise and to the point.

    Newly documented features are:

    • toTemplateReplacements
    • Nested property syntax

    Please review:

    Here's the diff-compare-view to the documentation branch:

    http://github.com/dandean/prototype/compare/documentation...doc_imp...

  • Samuel Lebeau

    Samuel Lebeau March 8th, 2010 @ 04:17 PM

    @Dan

    For String#interpolate, I guess that only referencing Template#evaluate as related to: is sufficient. The given example is good but the whole thing omits to reference the optional syntax argument. Why don't we simply use the existing documentation at http://prototypejs.org/api/string/interpolate ?

    As for Template#evaluate documentation, IMHO, it should be rewritten as such:

    initialize: function(first, last) {
      this.first = (first || '').strip();
      this.last = (last || '').strip();
    },
    toTemplateReplacements: function() {
      return {
        first: this.first,
        last:  this.last
      };
    },
    toString: function() {
      return this.first + " " + this.last;
    }
    

    so that the template instantiation looks like: new Template("Hi #{first} #{last}!");.
    I think it better illustrates the principle of returning the encapsulated instance variables to a plain object and doesn't mess with concatenating or stripping (the toTemplateReplacements logic you suggested really belongs to a toString implementation).
    BTW, I'd rather go for firstName and lastName but that's not really important...

  • Tobie Langel

    Tobie Langel March 8th, 2010 @ 04:19 PM

    Thanks. Looks good:

    A couple of comments:

    • line #6 & line #10: spelling, it's "concatenation" not "concatination".
    • line #22": Please avoid HTML elements in documentation.
    • line #23: I'd argue that it's the other way round: an object is applied to the template.

    Other than that, LGTM.

  • Tobie Langel

    Tobie Langel March 8th, 2010 @ 04:26 PM

    Samuel, you're missing the point of toTemplateReplacement as your example can be written like so:

    function Person(first, last) {
      this.first = (first || '').strip();
      this.last = (last || '').strip();
    }
    
    var joe = new Person('John', 'Doe');
    new Template("Hi #{first} #{last}!").evaluate(joe);.
    

    Dan, you're missing an explanation of how Template#evaluate(Array) works which was the whole point.

  • Samuel Lebeau

    Samuel Lebeau March 8th, 2010 @ 05:43 PM

    @Tobie: well you're right, it would work, but my assumption was that first and last were internal (say encapsulated, "private") and not meant to be accessible publicly but through accessors like getFirst and getLast, and that toTemplateReplacements job was to return a plain "struct" with those properties, but it's not clear as is…

    I'm not sure the initial example was really illustrating the principle of returning a plain object with "view-ready" properties, but maybe we should move forward and use it, anybody?

  • Dan Dean

    Dan Dean March 8th, 2010 @ 06:33 PM

    @Tobie:

    I'll fix that typo, remove the span, and fix the sentence on line 23. Also, this patch was committed before the "Array as argument" patch we were discussing the other day. I'll add that to this patch as well.

    @Samuel:

    I don't know how I overlooked the original String#interpolate docs -- they're much better than my example. I'll update this to match and recommit. Thanks for catching that.

    @all:

    How's this for the toTemplateReplacements example? It uses the toString method for name concatenation and makes the arguments and properties explicit.

    var Person = Class.create({
      initialize: function(firstName, lastName) {
        this.firstName = (firstName || '').strip();
        this.lastName = (lastName || '').strip();
      },
      toTemplateReplacements: function() {
        return { name: this.toString() };
      },
      toString: function() {
        return (this.firstName + " " + this.lastName).strip();
      }
    });
    

    I think there's value, though, in making code examples discrete and to the point, rather than production ready. I feel the goal should be to illustrate the principle of the feature in as clear terms as possible, and my example is starting to look bloated. Given that, how about this, which should address all of our concerns?

    function Person(firstName, lastName) {
      this.firstName = (firstName || '').strip();
      this.lastName = (lastName || '').strip();
    
      this.toTemplateReplacements = function() {
        return { name: (this.firstName + " " + this.lastName).strip() };
      }
    }
    

    Thanks for the thorough review guys -- I appreciate the input.

  • Dan Dean

    Dan Dean March 10th, 2010 @ 08:52 PM

    I've updated the docs to, hopefully, catch all of the concerns that were previously mentioned.

    Here's the commit compared to the current master.

    And the rendered results:

    Let me know if there are any other concerns, or just plain mistakes.

  • Samuel Lebeau

    Samuel Lebeau March 12th, 2010 @ 12:30 AM

    @Dan

    string.js is perfect.

    In template.js:

    • "your own objects" would be simpler than your own [[Class]] instances. Moreover, custom classes are not technically instances of the Class class.
    • This each call should be replaced by a collect with a return statement within the callback, the return value then being an array.
    • I guess this evaluate call would actually return "Hi Prince undefined!", which is not desired.

    Personally, I prefer the first example from your last comment, I agree with your argument but this one isn't really complicated and is far more Prototype-idiomatic than the final one.

    Would you be kind enough to compile all your commits into a single patch when we're done with reviewing ?

  • Dan Dean

    Dan Dean March 12th, 2010 @ 12:39 AM

    Sounds good. I'll try to get to this tonight. Thanks for the review Samuel!

  • hendry

    hendry March 25th, 2019 @ 01:18 PM

    we can easily understand the example of what it is mentioned here. The new documents are to template replacements and nested property syntax The template docs need mention of to louvre museum private tours template replacements to feature through the class and strings interpolate method.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Shared Ticket Bins

People watching this ticket

Pages