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

lib/template: allow range over number #1566

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

phenpessoa
Copy link
Contributor

@phenpessoa phenpessoa commented Sep 4, 2023

This PR adds the ability to range over a number (i.e. ints and uints) in template.

{{range $k, $v := 2}}{{$k}}{{$v}}{{end}}

Should print 0011

{{range 2}}{{.}}{{end}}

Should print 01

{{range 0}}{{else}}empty{{end}}

Should print empty

{{range -1}}{{else}}empty{{end}}

Should throw an error

@phenpessoa phenpessoa changed the base branch from master to dev September 4, 2023 19:06
Comment on lines 511 to 513
if intVal <= 0 {
break
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather raise a runtime error in this case. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine with me, but I think it might not be so user friendly if the value of n is a variable that the user does not know the value, like a count from DB
Perhaps < 0 could be a runtime error but = 0 could remain a simple break, WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think range 0 should be permitted, but negative is nonsensical.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Comment on lines 526 to 528
if intVal <= 0 {
break
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@SoggySaussages
Copy link
Contributor

SoggySaussages commented Sep 5, 2023

Why is this necessary when we already have seq? You can get the same result in each of your examples by prefixing seq 0 to each number. This method saves 5 characters for each call, is that the only difference? Would this confuse users and devalue the existing function? Maybe I'm misunderstanding something here.

@phenpessoa
Copy link
Contributor Author

phenpessoa commented Sep 5, 2023

It provides, imo, a better syntax. Also, it does not require allocating a slice, thus using less memory.
But yes, this PR is basically syntactic sugar.

Would this confuse users and devalue the existing function?

I don't think so. Seq still allows you to start from a value other than 0.

This PR is inspired on the proposal to range over n in go itself.

@ashishjh-bst ashishjh-bst merged commit fb9c075 into botlabs-gg:dev Sep 6, 2023
1 check passed
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.

4 participants