Implementing -first and -last and then multiple index counters in one loop iteration #18

Closed
psybert opened this Issue May 1, 2012 · 11 comments

Comments

Projects
None yet
2 participants
@psybert

psybert commented May 1, 2012

I successfully implemented #-first and #-last (same syntax used in other mustache extensions in other languages) using the delegate as described in counters.md, I also scoped them with the current key so that I could have nested -first/-last. But I had to do some hackery to get ^-first and ^-last to work (which seems really weird by the way setting a return value to nil to get it to display something, but hey it works). I had to import the private file for token and then write a category on the invocation to reference it in (i'm using ARC).

While doing all of this I noticed an obvious issue and that is having a counter to the array. Sure checking for NSArray lets me know that an array is about to start, but then we are making the assumption that we should increase the "index" every time {{index}} is called (or in my case -first). But what about if you call -first or index twice in the same loop iteration? How do you increment the counter without having to have exactly one placeholder per loop iteration?

While I'm not asking for you to implement something in the base code that isn't in the Mustache spec, I respect that, but I think we need more information through the delegate methods to write our own extensions without butchering up the main source.

Thoughts?

@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 2, 2012

Owner

The very reason for GRMustacheDelegate existence is to improve the expressivity of Mustache, while drawing a clear line between the Mustache spec and GRMustache extensions. Expressivity means that some want a zero-based index, others a one-based index, others want to distinguish odd and even indexes, etc. The -index, -first, -last "extensions" are nothing but a patch on a flat tire. I tried to push those stuff in "user-land", namely in the delegate.

I think the features/consistency you are requesting may be implemented via an NSArray-wrapper which holds a reference to its current iteration index: when GRMustache is about to render an NSArray, set the invocation's return value to this wrapper, which will respond to -first, -last, -index, -second, -odd, -multiple-of-three, etc.

I could add delegate methods dedicated to iterations, sure. Today I'm reluctant to it: this would be yet another patch, yet another proof that 1. Mustache has deep shortcomings, or 2. GRMustache tries too much to fight the spec. I can't decide between 1 and 2.

Those are my early thoughts. I'm quite willing to hear for feedback again.

Owner

groue commented May 2, 2012

The very reason for GRMustacheDelegate existence is to improve the expressivity of Mustache, while drawing a clear line between the Mustache spec and GRMustache extensions. Expressivity means that some want a zero-based index, others a one-based index, others want to distinguish odd and even indexes, etc. The -index, -first, -last "extensions" are nothing but a patch on a flat tire. I tried to push those stuff in "user-land", namely in the delegate.

I think the features/consistency you are requesting may be implemented via an NSArray-wrapper which holds a reference to its current iteration index: when GRMustache is about to render an NSArray, set the invocation's return value to this wrapper, which will respond to -first, -last, -index, -second, -odd, -multiple-of-three, etc.

I could add delegate methods dedicated to iterations, sure. Today I'm reluctant to it: this would be yet another patch, yet another proof that 1. Mustache has deep shortcomings, or 2. GRMustache tries too much to fight the spec. I can't decide between 1 and 2.

Those are my early thoughts. I'm quite willing to hear for feedback again.

@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 2, 2012

Owner

Also, what do you mean by the "hackery" around ^-first and ^-last? Don't you return a [NSNumber numberWithBool:] for those keys? If you had to hack in any way, you may well have found a bug: I'm very curious.

Owner

groue commented May 2, 2012

Also, what do you mean by the "hackery" around ^-first and ^-last? Don't you return a [NSNumber numberWithBool:] for those keys? If you had to hack in any way, you may well have found a bug: I'm very curious.

@psybert

This comment has been minimized.

Show comment Hide comment
@psybert

psybert May 2, 2012

Personally I think the code should give us the tools to implement whatever we want (however we want), so yes I don't need {{odd}} or whatever, but if I want to do it I can add it. Knowing what index of the NSArray I'm in will let me implement whatever I want. I can modify the source to do it myself, but then it becomes a pain to manage updates. I'm not sure if I follow how to keep track of the current iteration with an NSArray wrapper. If the delegate can expose more properties for developers to create their own tweaks, I think we'll be in good shape.

Here's the ^-first I was talking about ->

if ([invocation.key isEqualToString:@"-first"] && invocation.tokenType == GRMustacheTokenTypeInvertedSectionOpening) //some hackery to get the token type
    {
        int currentIndex = [[keyStackArrayCurrentIndex objectForKey:fullKey] intValue];

        if (currentIndex => 1)
        {
            invocation.returnValue = [NSNull null]; //displays whatever is in between {{^-first}} and {{/-first}}       
        }
        else 
        {
            invocation.returnValue = @"!"; //This text is never seen, but makes it so the ^first tag doesn't display anything
        }
    }

psybert commented May 2, 2012

Personally I think the code should give us the tools to implement whatever we want (however we want), so yes I don't need {{odd}} or whatever, but if I want to do it I can add it. Knowing what index of the NSArray I'm in will let me implement whatever I want. I can modify the source to do it myself, but then it becomes a pain to manage updates. I'm not sure if I follow how to keep track of the current iteration with an NSArray wrapper. If the delegate can expose more properties for developers to create their own tweaks, I think we'll be in good shape.

Here's the ^-first I was talking about ->

if ([invocation.key isEqualToString:@"-first"] && invocation.tokenType == GRMustacheTokenTypeInvertedSectionOpening) //some hackery to get the token type
    {
        int currentIndex = [[keyStackArrayCurrentIndex objectForKey:fullKey] intValue];

        if (currentIndex => 1)
        {
            invocation.returnValue = [NSNull null]; //displays whatever is in between {{^-first}} and {{/-first}}       
        }
        else 
        {
            invocation.returnValue = @"!"; //This text is never seen, but makes it so the ^first tag doesn't display anything
        }
    }
@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 3, 2012

Owner

I let aside the delegate extensions you need in this comment, and only address here the {{^-first}} hackery.

When GRMustache asks for the key -first, you should simply provide a boolean built with [NSNumber numberWithBool:]. It will be used to render, or not, the {{#-first}} and {{^-first}} sections, and the delegate doesn't have to know about the section type (I don't even want to know which private header you had to include in order to get the GRMustacheTokenTypeInvertedSectionOpening private value). My advice is to be straightforward, as below:

...
if ([invocation.key isEqualToString:@"-first"]) {
    int currentIndex = [[keyStackArrayCurrentIndex objectForKey:fullKey] intValue];
    invocation.returnValue = [NSNumber numberWithBool:(currentIndex == 0)];
}
...
Owner

groue commented May 3, 2012

I let aside the delegate extensions you need in this comment, and only address here the {{^-first}} hackery.

When GRMustache asks for the key -first, you should simply provide a boolean built with [NSNumber numberWithBool:]. It will be used to render, or not, the {{#-first}} and {{^-first}} sections, and the delegate doesn't have to know about the section type (I don't even want to know which private header you had to include in order to get the GRMustacheTokenTypeInvertedSectionOpening private value). My advice is to be straightforward, as below:

...
if ([invocation.key isEqualToString:@"-first"]) {
    int currentIndex = [[keyStackArrayCurrentIndex objectForKey:fullKey] intValue];
    invocation.returnValue = [NSNumber numberWithBool:(currentIndex == 0)];
}
...
@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 3, 2012

Owner

Now about the delegate extensions: would you provide some hints about the methods you're missing? For instance, would you write the signature of your ideal methods? We could discuss on that base.

Owner

groue commented May 3, 2012

Now about the delegate extensions: would you provide some hints about the methods you're missing? For instance, would you write the signature of your ideal methods? We could discuss on that base.

@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 3, 2012

Owner

Now, about the "wrapper": The technique is to replace the array with another one, whose objects themselves know how to respond to "-first", "-index", etc.

Here is some sample code:

template.mustache:

{{#people}}
- {{-index}} ({{#-first}}first{{/-first}}{{^-first}}not first{{/-first}}, {{#-last}}last{{/-last}}{{^-last}}not last{{/-last}}): {{.}}
{{/people}}
#import "GRAppDelegate.h"
#import "GRMustache.h"

@interface GRIndexedArrayElement : NSObject
- (id)initWithObjectAtIndex:(NSUInteger)index inArray:(NSArray *)array;
@end


#pragma mark

@interface GRAppDelegate()<GRMustacheTemplateDelegate>
- (NSString *)render;
@end

@implementation GRAppDelegate

@synthesize window = _window;

- (void)applicationDidFinishLaunching:(NSNotification *)aNotification
{
    NSLog(@"%@", [self render]);
}

- (NSString *)render
{
    NSArray *people = [NSArray arrayWithObjects:@"Alice", @"Bob", @"Craig", nil];
    NSDictionary *data = [NSDictionary dictionaryWithObject:people forKey:@"people"];
    GRMustacheTemplate *template = [GRMustacheTemplate templateFromResource:@"template" bundle:nil error:NULL];
    template.delegate = self;
    return [template renderObject:data];
}

- (void)template:(GRMustacheTemplate *)template willRenderReturnValueOfInvocation:(GRMustacheInvocation *)invocation
{
    if ([invocation.returnValue isKindOfClass:[NSArray class]]) {
        NSArray *array = invocation.returnValue;
        NSMutableArray *indexedArray = [NSMutableArray arrayWithCapacity:array.count];
        [array enumerateObjectsUsingBlock:^(id obj, NSUInteger idx, BOOL *stop) {
            [indexedArray addObject:[[GRIndexedArrayElement alloc] initWithObjectAtIndex:idx inArray:array]];
        }];
        invocation.returnValue = indexedArray;
    }
}

@end


#pragma mark

@interface GRIndexedArrayElement()
@property (nonatomic) NSUInteger index;
@property (nonatomic, strong) NSArray *array;
@end

@implementation GRIndexedArrayElement
@synthesize index=_index;
@synthesize array=_array;

- (id)initWithObjectAtIndex:(NSUInteger)index inArray:(NSArray *)array
{
    self = [super init];
    if (self) {
        self.index = index;
        self.array = array;
    }
    return self;
}

- (NSString *)description
{
    // support for the `.` implicit iterator
    return [[self.array objectAtIndex:self.index] description];
}

- (id)valueForKey:(NSString *)key
{
    if ([key isEqualToString:@"-index"]) {
        return [NSNumber numberWithUnsignedInteger:self.index];
    }

    if ([key isEqualToString:@"-first"]) {
        return [NSNumber numberWithBool:(self.index == 0)];
    }

    if ([key isEqualToString:@"-last"]) {
        return [NSNumber numberWithBool:(self.index == (self.array.count - 1))];
    }

    id object = [self.array objectAtIndex:self.index];
    return [object valueForKey:key];
}

@end

Output:

- 0 (first, not last): Alice
- 1 (not first, not last): Bob
- 2 (not first, last): Craig
Owner

groue commented May 3, 2012

Now, about the "wrapper": The technique is to replace the array with another one, whose objects themselves know how to respond to "-first", "-index", etc.

Here is some sample code:

template.mustache:

{{#people}}
- {{-index}} ({{#-first}}first{{/-first}}{{^-first}}not first{{/-first}}, {{#-last}}last{{/-last}}{{^-last}}not last{{/-last}}): {{.}}
{{/people}}
#import "GRAppDelegate.h"
#import "GRMustache.h"

@interface GRIndexedArrayElement : NSObject
- (id)initWithObjectAtIndex:(NSUInteger)index inArray:(NSArray *)array;
@end


#pragma mark

@interface GRAppDelegate()<GRMustacheTemplateDelegate>
- (NSString *)render;
@end

@implementation GRAppDelegate

@synthesize window = _window;

- (void)applicationDidFinishLaunching:(NSNotification *)aNotification
{
    NSLog(@"%@", [self render]);
}

- (NSString *)render
{
    NSArray *people = [NSArray arrayWithObjects:@"Alice", @"Bob", @"Craig", nil];
    NSDictionary *data = [NSDictionary dictionaryWithObject:people forKey:@"people"];
    GRMustacheTemplate *template = [GRMustacheTemplate templateFromResource:@"template" bundle:nil error:NULL];
    template.delegate = self;
    return [template renderObject:data];
}

- (void)template:(GRMustacheTemplate *)template willRenderReturnValueOfInvocation:(GRMustacheInvocation *)invocation
{
    if ([invocation.returnValue isKindOfClass:[NSArray class]]) {
        NSArray *array = invocation.returnValue;
        NSMutableArray *indexedArray = [NSMutableArray arrayWithCapacity:array.count];
        [array enumerateObjectsUsingBlock:^(id obj, NSUInteger idx, BOOL *stop) {
            [indexedArray addObject:[[GRIndexedArrayElement alloc] initWithObjectAtIndex:idx inArray:array]];
        }];
        invocation.returnValue = indexedArray;
    }
}

@end


#pragma mark

@interface GRIndexedArrayElement()
@property (nonatomic) NSUInteger index;
@property (nonatomic, strong) NSArray *array;
@end

@implementation GRIndexedArrayElement
@synthesize index=_index;
@synthesize array=_array;

- (id)initWithObjectAtIndex:(NSUInteger)index inArray:(NSArray *)array
{
    self = [super init];
    if (self) {
        self.index = index;
        self.array = array;
    }
    return self;
}

- (NSString *)description
{
    // support for the `.` implicit iterator
    return [[self.array objectAtIndex:self.index] description];
}

- (id)valueForKey:(NSString *)key
{
    if ([key isEqualToString:@"-index"]) {
        return [NSNumber numberWithUnsignedInteger:self.index];
    }

    if ([key isEqualToString:@"-first"]) {
        return [NSNumber numberWithBool:(self.index == 0)];
    }

    if ([key isEqualToString:@"-last"]) {
        return [NSNumber numberWithBool:(self.index == (self.array.count - 1))];
    }

    id object = [self.array objectAtIndex:self.index];
    return [object valueForKey:key];
}

@end

Output:

- 0 (first, not last): Alice
- 1 (not first, not last): Bob
- 2 (not first, last): Craig
@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 3, 2012

Owner

Also, please review the README section about how to embed GRMustache in your project. The more I read your sample code, the more I believe you have embedded raw GRMustache sources, along with private stuff that you should not use (as long as you plan your code to be robust). It's not your fault, since GRMustache is an old project, and that it used to be shipped as raw sources. Today, all you need in your project is GRMustache.h and a static library. Anything else should be removed.

Owner

groue commented May 3, 2012

Also, please review the README section about how to embed GRMustache in your project. The more I read your sample code, the more I believe you have embedded raw GRMustache sources, along with private stuff that you should not use (as long as you plan your code to be robust). It's not your fault, since GRMustache is an old project, and that it used to be shipped as raw sources. Today, all you need in your project is GRMustache.h and a static library. Anything else should be removed.

@psybert

This comment has been minimized.

Show comment Hide comment
@psybert

psybert May 4, 2012

Gwendal, thanks for the sample. It really clears up the concerns I had. I was clearly going about it a different way. Your sample here works really well. Basically if I understand correctly grmustache uses nsobjects valueforkey quite a bit so by inserting your own class you can intercept the method and return whatever you want.

I also fixed up the references. I added the private headers when I was going about my hacking, but now they are unnecessary.

I had a couple of other (smaller) questions that are unrelated to this issue. Should I email them, ask them here or ask in another issue item? Thanks again.

psybert commented May 4, 2012

Gwendal, thanks for the sample. It really clears up the concerns I had. I was clearly going about it a different way. Your sample here works really well. Basically if I understand correctly grmustache uses nsobjects valueforkey quite a bit so by inserting your own class you can intercept the method and return whatever you want.

I also fixed up the references. I added the private headers when I was going about my hacking, but now they are unnecessary.

I had a couple of other (smaller) questions that are unrelated to this issue. Should I email them, ask them here or ask in another issue item? Thanks again.

@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 4, 2012

Owner

Thanks, psybert, I'm happy this sample code has ben helpful. I must admit, however, that it has big shortcomings, and that the subject is not totally closed yet . Precisely, I've seen at least two issues : 1. BOOL properties of objects in the array can not any longer control sections, since GRIndexedArrayElement does not expose any BOOL property of its own (read the booleans.md guide for more information). 2. Missing keys will raise exceptions that GRMustache will not catch, since they do not come from the object keys are requested from (see context_stack.md guide for more information).

Hence you have found a real issue here. I'm very thankful for that. I'll have to address it seriously. Meanwhile, I'm happy the solution we came up with so far solves your issues (I let you close the ticket if you're happy).

About your other questions : please open issues, since other users may have interest in reading our conversations.

Owner

groue commented May 4, 2012

Thanks, psybert, I'm happy this sample code has ben helpful. I must admit, however, that it has big shortcomings, and that the subject is not totally closed yet . Precisely, I've seen at least two issues : 1. BOOL properties of objects in the array can not any longer control sections, since GRIndexedArrayElement does not expose any BOOL property of its own (read the booleans.md guide for more information). 2. Missing keys will raise exceptions that GRMustache will not catch, since they do not come from the object keys are requested from (see context_stack.md guide for more information).

Hence you have found a real issue here. I'm very thankful for that. I'll have to address it seriously. Meanwhile, I'm happy the solution we came up with so far solves your issues (I let you close the ticket if you're happy).

About your other questions : please open issues, since other users may have interest in reading our conversations.

@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 26, 2012

Owner

GRMustache4 removes all the caveats of the solution using GRIndexedArrayElement. Considering adding this snippet to the documentation guides.

Owner

groue commented May 26, 2012

GRMustache4 removes all the caveats of the solution using GRIndexedArrayElement. Considering adding this snippet to the documentation guides.

groue added a commit that referenced this issue May 26, 2012

@groue

This comment has been minimized.

Show comment Hide comment
@groue

groue May 26, 2012

Owner

The recommended solution is now written in a guide: https://github.com/groue/GRMustache/blob/master/Guides/sample_code/indexes.md (requires GRMustache4)

Owner

groue commented May 26, 2012

The recommended solution is now written in a guide: https://github.com/groue/GRMustache/blob/master/Guides/sample_code/indexes.md (requires GRMustache4)

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