-
Notifications
You must be signed in to change notification settings - Fork 161
Specify sizes and ranges of DAP integers #583
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
base: main
Are you sure you want to change the base?
Conversation
- Generally types/IDs that were only specified as `integer`s are now int32s. I only defined minimums on those that previously had ">0" behavior - Lines and columns are uint64 since they're only ever >0 and we should be able to express more than (u)int32-sized files. With these and other 64-bit types, I set the `minimum` and `maximum` to the max safe floating point integers so as to ensure compatibility with floating-point-based runtimes, like JavaScript and Lua (<=5.2) - variablesReference is defined as uint32 because we consistently define behavior as "when variablesReference is >0" and "<0" is undefined - systemProcessId is an int32. While research shows there are some exotic systems where it _can_ be an int64 (Solaris) it is practically still an int32 (even 64 bit Linux limits to 2^22) and we have already defined it as such elsewhere - Memory-related offset/count is defined as int64 - namedVariables/indexedVariables are counts, but they were previously specified as having a max value of `2147483647` so I kept them as int32's with a `minimum` value in the schema I would greatly appreciate folks that use various DAP clients to try this out and let me know if you run into issues or see things that should be changed. I'll keep this PR open for a while before merging it to gather feedback. Closes #422 Closes #551
51fc6f2 to
d97ee9c
Compare
|
@connor4312 the changes seem ok to me/Dart. Did/do they result in any changes to the VS Code debug client code? (if so, it would be useful to see them and/or test with them). |
Not immediately, since we're TypeScript and all |
osiewicz
left a comment
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.
On Zed's side, we're ok with these changes as well. We control our DAP types library, so worst case we'll just adjust. For us, the biggest benefit of this change is having the ranges specified in spec (whatever they are), as we've dealt with quite a few DAP servers that had a differing ranges.
|
The sizes seem reasonable to me. Thanks |
|
Looks good to me. |
integers are now int32s.minimumandmaximumto the max safe floating point integers so as to ensure compatibility with floating-point-based runtimes, like JavaScript and Lua (<=5.2)2147483647so I kept them as int32's with aminimumvalue in the schemaI would greatly appreciate folks that use various DAP clients to try this out and let me know if you run into issues or see things that should be changed. I'll keep this PR open for a while before merging it to gather feedback.
cc @osiewicz @mfussenegger @DanTup @octo @puremourning @gregg-miskelly
Closes #422
Closes #551