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

if then else and chained block syntax #8489

Closed
JaroslavTulach opened this issue Dec 7, 2023 · 8 comments · Fixed by #8671
Closed

if then else and chained block syntax #8489

JaroslavTulach opened this issue Dec 7, 2023 · 8 comments · Fixed by #8671
Assignees

Comments

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Dec 7, 2023

Assuming that

    main =
        pairs = (if t then a else b) . to_vector . sort

and

    main =
        pairs = if t then a else b
            . to_vector
            . sort

shall be the same. They are not right now on 2023.2.1-nightly.2023.12.7

@somebody1234
Copy link
Contributor

@JaroslavTulach i think you messed up the link?

@JaroslavTulach
Copy link
Member Author

@kazcw what do you think? As far as I can see in debugger the chained code is equivalent to b . to_vector . sort - e.g. the chain is only applied to the b and not to the result of the whole if_then_else.

@JaroslavTulach JaroslavTulach removed their assignment Dec 7, 2023
@JaroslavTulach
Copy link
Member Author

There are three tests. Two work, but testIfAndBlockSyntax fails. Is that correct or wrong, @kazcw?

@kazcw
Copy link
Contributor

kazcw commented Dec 7, 2023

I'm looking for a consistent rule that would make this work. Here's a candidate:

The body of an else segment is either:

  • The rest of the line (if non-empty)
  • A following child block (if there is no expression following the else on the same line)

This is a change from current behavior, in which if the else has in-line content, any following child block belongs to the else branch.

This would also affect other block types, e.g. this:

    main =
        value = if t then f1 else f2
            y

Would be equivalent to this:

    main =
        value = (if t then f1 else f2)
            y

IMO it minimizes confusion to treat all block types the same way in this regard.

What do you think, @JaroslavTulach? If we shall do it, I can use the AST-comparison script to check for any existing code relying on the old behavior.

@JaroslavTulach
Copy link
Member Author

The rest of the line (if non-empty)
A following child block (if there is no expression following the else on the same line)

+1

@enso-bot
Copy link

enso-bot bot commented Feb 22, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-02-21):

Progress: - rust parser: https://github.com/enso-org/enso/pull/8671/files#r1497057518

Next Day: Rust parser, Extension methods for Java

GitHub
Pull Request Description Attempt to fix #8489 by

adding test describing current behavior: c5bef79
another test showing desired Group-like behavior: 1c722f4
changing the first test to behave like t...

@enso-bot
Copy link

enso-bot bot commented Feb 23, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-02-22):

Progress: - chained if_then_else re-review: #8671 (review)

Next Day: Extension methods for Java

@enso-bot
Copy link

enso-bot bot commented Feb 24, 2024

Jaroslav Tulach reports a new STANDUP for yesterday (2024-02-23):

Progress: - bookclubbing

Next Day: Extension methods for Java

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

Successfully merging a pull request may close this issue.

3 participants