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

Add more tests for type arguments in object patterns and remove a TODO. #1401

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

munificent
Copy link
Member

I'm honestly not sure what the TODO was even trying to say. But I think the formatter behaves like we want here so there's nothing TODO as far as I can tell. But this corner did seem a little under tested, so added some more.

I'm honestly not sure what the TODO was even trying to say. But I think
the formatter behaves like we want here so there's nothing TODO as far
as I can tell. But this corner did seem a little under tested, so added
some more.
Copy link
Member

@kallentu kallentu left a comment

Choose a reason for hiding this comment

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

Haha, that TODO is on me.
I wasn't sure if that's what we wanted.
The old test expected output was which I thought was much better than what's on line 172.

if (obj
    case LongClassName<
      First,
      Second
    >()) {
  ;
}

@munificent
Copy link
Member Author

Oh, you're right! The output is wrong and is missing some indentation.

This must be an issue with collapsible indentation thing I did. I'll look into that.

A block indent shouldn't be able to partially eat an expression-sized
collapsible indentation. The only time we want to actually collapse is
when we have two equivalent expression-sized indentations.
@munificent
Copy link
Member Author

OK, I think I have it producing the right output now. The whole "collapsible indentation" thing still feels a little iffy to me, but it seems to get the job done.

PTAL, thanks!

@munificent munificent merged commit 9dd81d6 into main Feb 22, 2024
7 checks passed
@munificent munificent deleted the object-pattern-type-argument-tests branch February 22, 2024 21:18
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.

2 participants