Surge-ee / SurgerEE

A collection of simple tags for expressionengine that make template development easier.
Mozilla Public License 2.0
37 stars 8 forks source link

Added an {tag} to the Loop function which starts at zero. #14

Closed cwcrawley closed 11 years ago

robsonsobral commented 11 years ago

May I suggest you something?

At my commits, I prefixed surgeree tags to avoid parse conflicts. EECMS template parse doesn't respect child variables of nested plugins. Per example, the {count} tag is parsed by the first plugin being parsed, no matter if it is child of the second or third nested plugin.

{index} is a common name. Others plugins can use this name too. So, I suggest you to use {surg:loop:index}. But this is up to @EpocSquadron . He is the boss here! We need to follow a standard.

Thanks for your attention, guys!

cwcrawley commented 11 years ago

Yes, I totally agree.

Renaming and either having {surgeree_index} and {surgeree_count} would be a great update - alternatively, allow for a prefix="" modifier parameter. When blank it retains the natural tags (as they are now), but prefix="surgeree"} could either return {surgeree_index} or {surgeree:index} depending how you tweak the return array.

Regards,

Carl

On 15 Jan 2013, at 14:18, robsonsobral notifications@github.com wrote:

May I suggest you something?

At my commits, I prefixed surgeree tags to avoid parse conflicts. EECMS template parse doesn't respect child variables of nested plugins. Per example, the {count} tag is parsed by the first plugin being parsed, no matter if it is child of the second or third nested plugin.

{index} is a common name. Others plugins can use this name too. So, I suggest you to use {surgindex}. But this is up to @EpocSquadron . He is the boss here! We need to follow a standard.

Thanks for your attention, guys!

— Reply to this email directly or view it on GitHub.

robsonsobral commented 11 years ago

I've been using {surg:name_of_the_function:name_of_the_variable} to avoid conflicts between nested surgeree tags.

What do you think, guys?

For replace default {count} and {total_results} variables, we should use parse_variables_row() instead of parse_variables() and set this values for ourselves. In some cases could be little performance impacts of this approach: count the length of the array and add, one by one, the total_results item. But, maybe, this worth the cost.

Again, is up to @EpocSquadron to decide.

EpocSquadron commented 11 years ago

I think this is a great idea. I like the surgeree:function:var nomenclature. I would rather not have another parameter for the prefix and this solves the issue nicely. I'll look into parse_variables_row. In the mean time, it's going to take some time to process these pull requests cleanly.

robsonsobral commented 11 years ago

My mistake! We don't need to use parse_variables_row(). parse_variables() will be fine.

EpocSquadron commented 11 years ago

I've merged in the additional change for this pull request via cherry-pick for a cleaner history. I'll be standardizing the variable nomenclature separately.

We will stay away from parse_variables_row for now, since the count/total_results behavior is expected.