-
Notifications
You must be signed in to change notification settings - Fork 45
Started omitting instructions if they are null. #221
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
Conversation
|
This is a pretty tricky way to hide a bug. I wonder if using extension types for InitializationResult is actually buying us anything. |
|
I'm not sure what the release protocol is for this, do you want me to update the changelog.md and bump the version too? |
| }) { | ||
| final map = { | ||
| 'protocolVersion': protocolVersion.versionString, | ||
| 'capabilities': serverCapabilities, | ||
| 'serverInfo': serverInfo, | ||
| }; | ||
| if (instructions != null) { | ||
| map['instructions'] = instructions; | ||
| } | ||
| return InitializeResult.fromMap(map); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }) { | |
| final map = { | |
| 'protocolVersion': protocolVersion.versionString, | |
| 'capabilities': serverCapabilities, | |
| 'serverInfo': serverInfo, | |
| }; | |
| if (instructions != null) { | |
| map['instructions'] = instructions; | |
| } | |
| return InitializeResult.fromMap(map); | |
| } | |
| }) => InitializeResult.fromMap({ | |
| 'protocolVersion': protocolVersion.versionString, | |
| 'capabilities': serverCapabilities, | |
| 'serverInfo': serverInfo, | |
| 'instructions': ?instructions, | |
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if our language version doesn't support that, we can do if (instructions != null) 'instructions': instructions,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We needed 3.8 for ?instructions, done.
Yes, bump the version to |
done |
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
fixes #196
This makes sure that the map for the InitializationResult doesn't include a
instructionsentry if the value is null.Contribution guidelines:
dart format.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.