FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Support Show as Binary by mylog00 · Pull Request #429 · microsoft/java-debug · GitHub

Support Show as Binary - #429

Open
Dmitrii Kniazev (mylog00) wants to merge 6 commits into
microsoft:mainfrom
mylog00:variable_binary_format
Open

Support Show as Binary#429
Dmitrii Kniazev (mylog00) wants to merge 6 commits into
microsoft:mainfrom
mylog00:variable_binary_format

Conversation

Copy link
Copy Markdown

Hi there!
This is PR for allow represent variables in binary forma.
I think it may be helpful for vscode-java-debug issue #1078
I've added new protocol parameter 'formatType' should contains variable format such as [BIN, OCT, HEX, DEC].
I've remain "showHex" for backward compatibility and can be delete later when PR to vscode-java-debug will be merged.
If you are not interested in this changes fill free to close this PR :)

Deleted user (ghost) commented Jul 24, 2022
edited by ghost
Loading

Copy link
Copy Markdown


All CLA requirements met.

Comment on lines 27 to 28
@Deprecated
public boolean hex;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

hex is part of the specification: https://microsoft.github.io/debug-adapter-protocol/specification#Types_ValueFormat, what's the rationale for deprecating this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Because of new functionality we can format values to different formats now, beside hex. And I added new property ValueFormat#type instead of ValueFormat#hex to support that possibility. I assumed we can delete ValueFormat#hex in future with no harm because of no need and marked hex as deprecated. Just tell me if I did something wrong and I'll fix it. :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Dmitrii Kniazev (@mylog00) The data types under the folder com/microsoft/java/debug/core/protocol are generated from the DAP spec, we can't just change it on debugger side.

It's OK to extend the debug settings to support more format types, but remember to support the old valueFormat.hex as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I've removed @Deprecated annotation and added doc to new property ValueFormat#type. Support of the old valueFormat.hex already was in my code for backward compatibility.
Should I create PR to DAP or do something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

When contributing to the DAP, you need to open an issue to discuss the proposal first. Only if the proposal is accepted, then open a PR.

A quick search and found a duplicated issue for more formatting options. microsoft/debug-adapter-protocol#197

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Got it. Thanks)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi! If there is no other issue could you approve this PR?)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL