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.

Sign up for GitHub

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

Jump to bottom

13097: Fix FewerBraces eta expansion error #14092

Closed
wants to merge 5 commits into from
Closed

13097: Fix FewerBraces eta expansion error #14092

wants to merge 5 commits into from

Conversation

gagandeepkalra
Copy link
Contributor

@gagandeepkalra gagandeepkalra commented Dec 12, 2021 •
edited
Loading

resolves 13097

@griggt

@gagandeepkalra gagandeepkalra changed the title wip: 13097 fewer braces eta expansion 13097: Fix FewerBraces eta expansion error Feb 9, 2022
@griggt griggt self-assigned this Feb 11, 2022
@gagandeepkalra
Copy link
Contributor Author

gagandeepkalra commented Mar 1, 2022 •
edited
Loading

This is ready for review. @griggt

@ckipp01
Copy link
Member

ckipp01 commented May 10, 2023

Hey @gagandeepkalra, I'm sorry this has taken so long to get a review on 😆 Is this something you're still willing to push over the finish line? If so, can you rebase and then we can get you a review on this.

@som-snytt
Copy link
Contributor

The ticket (which was mine, thanks for addressing) saw other duplicates and finally a fix that was merged. I haven't compared the solutions to see if this PR offers something else to preserve. What year is this anyway?

@ckipp01
Copy link
Member

ckipp01 commented May 10, 2023

The ticket (which was mine, thanks for addressing) saw other duplicates and finally a fix that was merged. I haven't compared the solutions to see if this PR offers something else to preserve. What year is this anyway?

haha, no worries. Since the original issue is closed now I'll go ahead and close this one as well. Thanks again for creating this @gagandeepkalra, I'm sure it helped with the prs that fixed the issue. @som-snytt if it turns out that something is still missing from your report or this brings in something else additionally, please to cherry pick it or report it and we can re-address.

@ckipp01 ckipp01 closed this May 10, 2023
@som-snytt
Copy link
Contributor

I will try to add the repl test, for fun and profit. I will co-author gagandeepkalra . I wonder what he is doing these days. Oh, I see Seth follows him, so I'll ask Seth.

@som-snytt som-snytt mentioned this pull request May 10, 2023
Merged
ckipp01 pushed a commit that referenced this pull request May 12, 2023
@som-snytt @gagandeepkalra
Test require indent after colon at EOL in REPL (#17452)
01e7c73 
Follow-up #16466

with test from #14092

which was sadly ignored.

Co-authored-by: Gagandeep kalra <gagandeepkalra1994@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@griggt griggt Awaiting requested review from griggt

Assignees

@griggt griggt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fewerBraces allows empty indentation region
4 participants
@gagandeepkalra @ckipp01 @som-snytt @griggt