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

Update spec details, added helper init functions #18

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

FastestMolasses
Copy link
Contributor

Updated some fields to match the more recent spec, as well as added some helper init functions to make it easier to create LSPRanges from language server responses.

@@ -167,7 +167,7 @@ public struct CompletionItem: Codable, Hashable, Sendable {
public let filterText: String?
public let insertText: String?
public let insertTextFormat: InsertTextFormat?
public let textEdit: TextEdit?
public let textEdit: TwoTypeOption<TextEdit, InsertReplaceEdit>?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -28,7 +28,7 @@ public struct InitializeParams: Codable, Hashable, Sendable {
public let workspaceFolders: [WorkspaceFolder]?

public init(
processId: Int,
processId: Int?,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mattmassicotte
Copy link
Contributor

Amazing fixes! Did you find this due to language servers complaining?

Two comments about the new initializers.

I get what you are thinking with the from label for the first parameter. But it does feel it overlaps with the Codable stuff a bit, no? What do you think about keeping it start?

Second, don't these all have the potential to create invalid end values? Can you help me understand where this could be helpful?

@mattmassicotte
Copy link
Contributor

I'd like to get this in. Let's get this figured out!

@mattmassicotte
Copy link
Contributor

@FastestMolasses I open this isn't too disruptive, but I wanted to accept these changes without stealing credit. To do that, but without adding the additional LSPRange API changes, I pushed to your main. I really hope this doesn't cause problems!

@mattmassicotte mattmassicotte merged commit ac76fcc into ChimeHQ:main Apr 1, 2024
4 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