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

please support range syntax #326

Closed
endink opened this issue Jan 11, 2021 · 18 comments
Closed

please support range syntax #326

endink opened this issue Jan 11, 2021 · 18 comments

Comments

@endink
Copy link
Contributor

endink commented Jan 11, 2021

array define : [1,2,3,4,5]
range define: [1..5]

This syntax is useful in an inline environment to effectively shorten script lengths.

Some language support it like kotlin, groovy...etc

@geseq
Copy link
Collaborator

geseq commented Feb 10, 2021

This is actually not a bad idea. @d5 any thoughts on this? I wouldn't mind making some time for this.

@d5
Copy link
Owner

d5 commented Feb 10, 2021

Sure. Can we discuss more details on the syntax first?

@geseq
Copy link
Collaborator

geseq commented Feb 10, 2021

Just looking up how this is done in other languages now but from a cursory overview I’m leaning towards the python style range builtin function rather than changing the array syntax.

https://www.w3schools.com/python/ref_func_range.asp

@d5
Copy link
Owner

d5 commented Feb 10, 2021

Haha. Yeah I like that!

@d5
Copy link
Owner

d5 commented Feb 10, 2021

So the new builtin function will return an array of ints?

@geseq
Copy link
Collaborator

geseq commented Feb 10, 2021

Yes, an array of ints from 0 to a n-1 If one argument is provided, start to end-1 with two args, or start to end-1 with step Increment with three args

@endink
Copy link
Contributor Author

endink commented Feb 10, 2021

I think if it's just a built-in, function, and we already have enough APIs to implement it ourselves, the built-in bracket syntax support would be much easier to read for line expression scenarios

@geseq
Copy link
Collaborator

geseq commented Feb 10, 2021

I’m not sure I agree that brackets add something to expressiveness that a builtin range function doesn’t.

It’s also IMO an unnecessary addition to the syntax which I’d rather leave as is unless there’s a really good reason to change it.

Additionally, the bracket syntax is limited. It doesn’t easily allow you to add a step size.

@endink
Copy link
Contributor Author

endink commented Feb 10, 2021

I use Tengo in a database sharding proxy project, it allows me to simplify some definitions, such as database node definitions: DS_1, DS_2, DS_3. If I have this syntax I can say DS_{[1..3]}, this makes the configuration file simple and easy to read. If it's a function: DS_{rang(1,3)}, I don't think it's as clear so.

In my project, the more complex
inline expression:
ds_{[1..9]}.table_{[1..5]}

This expression is in the configuration file, so avoiding the function is easier to read.

@endink
Copy link
Contributor Author

endink commented Feb 10, 2021

BTW: Because missing this syntax, I implemented a "range" function on my project, and this issue comes from a feeling that the configuration file is ugly.

@geseq
Copy link
Collaborator

geseq commented Feb 10, 2021

I suppose we can achieve something like that with [start:step:end] and [start:end] syntax but I’m not convinced of the need for it. While your example expression does not look pretty it is readable. I’m reluctant to change the language syntax to make this pretty.

@d5 thoughts?

@d5
Copy link
Owner

d5 commented Feb 10, 2021

Yeah I agree with @geseq. It's a nice-to-have feature and not worth all the extra efforts.

@geseq
Copy link
Collaborator

geseq commented Feb 10, 2021

In that case @endink would you be willing to create a PR with your implementation of the range function? If not I’ll work on implementing it.

@endink
Copy link
Contributor Author

endink commented Feb 11, 2021

@geseq Alright, these days are the Chinese New Years. If need, I can provide this PR after this holiday .

@geseq
Copy link
Collaborator

geseq commented Feb 23, 2021

@endink any luck with the PR? If you prefer I can work on this as well.

@endink
Copy link
Contributor Author

endink commented Feb 24, 2021

@geseq The Chinese New Year ends on February 27th, wait two days for me to do it or you do it. I'm fine with it anyway.You decide

@geseq
Copy link
Collaborator

geseq commented Feb 24, 2021

Oh no worries! I didn’t know that. Take your time.

@endink
Copy link
Contributor Author

endink commented Mar 2, 2021

@geseq Pr has been commited, and the specific usage can be seen in unit tests

range(start, stop[, step])

usage examples:

range(0,0) ----> [0]
range(0,0) ----> []
range(0, 5) -----> [0, 1, 2, 3, 4]
range(0, 10, 3) -----> [0, 3, 6, 9]
range(0, -10, 2) -----> [0, -2, -4, -6, -8]

BTW: I don't think a single parameter is a good syntax because its semantics are not clear, so my implementation doesn't include it ( like: range(5) ----> [0,1,2,3,4] )

d5 pushed a commit that referenced this issue Mar 2, 2021
* builtin range function #326

* change empty range logic

* fix unit test error message

* fix github env (#329)

* fix ErrInvalidRangeStep comments

* fix github env (#329)

* builtin range function #326

* change empty range logic

* fix unit test error message

* fix ErrInvalidRangeStep comments

* fix lint

Co-authored-by: geseq <5458743+geseq@users.noreply.github.com>
@geseq geseq closed this as completed May 24, 2021
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

No branches or pull requests

3 participants