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

[Feature] Handle reverse range #471

Open
mks-h opened this issue Sep 8, 2024 · 9 comments
Open

[Feature] Handle reverse range #471

mks-h opened this issue Sep 8, 2024 · 9 comments

Comments

@mks-h
Copy link
Member

mks-h commented Sep 8, 2024

The range expression (1..12/1..=12) can be written in reverse, like 12..1/12..=1. This will compile just fine, but on runtime a reversed range like this will not expand to anything (echo 12..1 outputs an empty string). For the reference, the range may include a variable, so we cannot always know whether the range is reversed or not.

First of all, we need to decide whether to allow reversed range expressions (12..1 => seq 12 -1 1), or not. And if not, we can think about providing an alternative syntax for them.

Whether we allow it or not, we can detect that a range is reversed in two ways:

  1. At compile time, when using numeric literals (12..1)
  2. If there's a variable, with a runtime conditional before doing seq

If we decide to forbid reverse range, we should make the runtime-checked range failable and error out when it is indeed reversed. If it's known to be reversed at compile time, the compiler should error out.

If we decide not to forbid the reverse range, there's no need to do anything with the literal reversed ranges like 12..1, as it is obvious that the range is reversed. But if there's a variable, the programmer might assume it is going to be a forward range, despite it possibly being reversed. So we should probably find a way to handle both cases in code. On the other hand, this might easily make it annoying to write ranges.

@mks-h mks-h added enhancement New feature or request feature proposal labels Sep 8, 2024
@b1ek
Copy link
Member

b1ek commented Sep 9, 2024

i don't see why we shouldn't allow those.

i guess it would be wise to implement it as a stdlib function rather than as a language feature, since we'd have to do a runtime check there

@Mte90 Mte90 changed the title Handle reverse range [Feature] Handle reverse range Sep 9, 2024
@Mte90 Mte90 added the syntax label Sep 9, 2024
@Mte90
Copy link
Member

Mte90 commented Sep 9, 2024

I think that should be in the compiler as range it's there

With a check if it should be reversed, doesn't make so much sense to split the feature in the stdlib.

@b1ek
Copy link
Member

b1ek commented Sep 10, 2024

With a check if it should be reversed, doesn't make so much sense to split the feature in the stdlib.

if the range uses variables as start and end, we couldn't tell that on compile time

@Mte90
Copy link
Member

Mte90 commented Sep 10, 2024

if the range uses variables as start and end, we couldn't tell that on compile time

that's a problem, so we need a way to inject a tiny bash function that do the reverse I guess.

@b1ek
Copy link
Member

b1ek commented Sep 11, 2024

if the range uses variables as start and end, we couldn't tell that on compile time

that's a problem, so we need a way to inject a tiny bash function that do the reverse I guess.

we never did that before, and it seems like a very very bad idea. if we are doing runtime checks, it has to be in stdlib

@b1ek
Copy link
Member

b1ek commented Sep 11, 2024

we could also allow syntax ranges like 0..9 with numeric literals only, and add a seq() function in the stdlib

@Mte90
Copy link
Member

Mte90 commented Sep 11, 2024

so it is the case to open a ticket to discuss what to do and close this PR?

@mks-h
Copy link
Member Author

mks-h commented Sep 11, 2024

@Mte90 this is an issue ticket, not a PR

@Mte90
Copy link
Member

Mte90 commented Sep 11, 2024

Sorry I confused with #469 where we are discussing similar things.

@Ph0enixKM Ph0enixKM added this to the Amber 0.5.0-alpha milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants