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

Refactorings to complement large PR in LanguageServerProtocol #9

Merged
merged 17 commits into from
Nov 28, 2023

Conversation

koliyo
Copy link
Contributor

@koliyo koliyo commented Sep 15, 2023

This should be updated to match whatever is decided in the main PR:
ChimeHQ/LanguageServerProtocol#8

@mattmassicotte
Copy link
Contributor

I think we can finally revisit this now!

@koliyo
Copy link
Contributor Author

koliyo commented Nov 24, 2023

Building for debugging...
/Users/nils/Work/hylo-lsp/LanguageClient/Tests/LanguageClientTests/ServerTests.swift:46:3: error: global function 'XCTAssertEqual(_:_:_:file:line:)' requires that 'MockServer.ClientMessage' conform to 'Equatable'
                XCTAssertEqual(messages, [
                ^
Swift.Array:1:11: note: requirement from conditional conformance of '[MockServer.ClientMessage]' to 'Equatable'
extension Array : Equatable where Element : Equatable {
          ^
error: fatalError

Note sure what to do about this, if we should make ClientMessage Equatable or refactor test?

@mattmassicotte
Copy link
Contributor

Now that ClientRequest takes captures closures, it is not longer possible to make an equality check. So, it won't be possible to make MockServer.ClientMessage conform to equatable. We'll have to refactor the test.

@mattmassicotte
Copy link
Contributor

I think it will be a little annoying, but it should be possible to do a similar check by pulling out the non-closure data and comparing that.

@koliyo
Copy link
Contributor Author

koliyo commented Nov 24, 2023

Ok, I will leave that to you (?) 😅
Let me know if you need some updates from my side!

@mattmassicotte
Copy link
Contributor

Yeah that makes sense. I'd actually like to look a little more closely at this PR too. Next week!

@mattmassicotte
Copy link
Contributor

Also, Nils, I just wanted to thank you for all your work here. I'm amazed!

Since I don't have other ways to get in touch with you, I just wanted to make you aware that I have a Discord server set up specifically for work on these open source projects. Totally optional, but in case that's your kind of thing: https://discord.gg/esFpX6sErJ

@koliyo
Copy link
Contributor Author

koliyo commented Nov 24, 2023

Well thank you! I'm glad I did not have to start from zero, and this was a great opportunity to get familiar with swift!

I'll join the discord. I probably won't be very active, but nice to be part of the community!

@mattmassicotte mattmassicotte marked this pull request as ready for review November 28, 2023 16:48
@mattmassicotte mattmassicotte merged commit eda2fdc into ChimeHQ:main Nov 28, 2023
3 checks passed
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