Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add optional 'start' param to '{@idx}' #80

Closed
wants to merge 1 commit into from

Conversation

idoa01
Copy link

@idoa01 idoa01 commented Jun 2, 2014

currently, if you want to use {$idx} as a 1-based index, you need to do something like this:

{@math key="{$idx}" method="add" operand="1" /}

with this pull request, the idx helper gets a new optional param start which will allow doing something like:

{@idx start="1"}{.}{/idx}

@kate2753
Copy link
Contributor

Interesting idea @idoa01 ! Seems handy to support this.

I think param name start might be unambiguous. To me it seems to mean "skip all indexes before start and output >=start index" rather than "add this value to index". Maybe I'm overthinking this though :)

@rragan @jimmyhchan @prashn64 do you have an opinion on this?

@rragan
Copy link
Contributor

rragan commented Jul 12, 2014

Would be simpler for base 1 vs 0 list number generation.

Rich
Sent from my iPhone

On Jul 11, 2014, at 9:25 PM, "Kate Odnous" <notifications@git.luolix.topmailto:notifications@github.com> wrote:

Interesting idea @idoa01https://github.com/idoa01 ! Seems handy to support this.

I think param name start might be unambiguous. To me it seems to mean "skip all indexes before start and output >=start index" rather than "add this value to index". Maybe I'm overthinking this though :)

@rraganhttps://github.com/rragan @jimmyhchanhttps://github.com/jimmyhchan @prashn64https://github.com/prashn64 do you have an opinion on this?


Reply to this email directly or view it on GitHubhttps://github.com//pull/80#issuecomment-48784790.

@sixlettervariables
Copy link

Perhaps "offset"? Would help with pagination as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants