Skip to content

Changed formatting of tooltips for numeric types#453

Open
3fbaea00 wants to merge 3 commits into
dnSpyEx:masterfrom
3fbaea00:master
Open

Changed formatting of tooltips for numeric types#453
3fbaea00 wants to merge 3 commits into
dnSpyEx:masterfrom
3fbaea00:master

Conversation

@3fbaea00

Copy link
Copy Markdown

Problem

Poorly designed tooltips for constants of numeric type.

  1. No need for an octal number system.
  2. Formatting negative numbers with sign preservation makes no sense.
  3. Hex representation for non-integer numbers is of poor use, instead a byte-array representation with fixed length, 4 bytes for float and 8 bytes for double is suggested.

Proposal

image image image image image

@ElektroKill ElektroKill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey,

I agree with points 1 and 2, but I think we should keep the underscores for base 2.

As for point number 3, I prefer the existing way of formatting, using a hexadecimal number to represent the raw data of the floating-point number. I think it's easier to read, especially due to the grouping.

Comment thread dnSpy/dnSpy/Documents/Tabs/DocViewer/ToolTips/NumberUtils.cs
@3fbaea00

Copy link
Copy Markdown
Author

Regarding point number 3, does it mean that I should add a second Raw representation, or should I just revert it to the 0xXXXX_XXXX_XXXX_XXX format?

@ElektroKill

Copy link
Copy Markdown
Member

Regarding point number 3, does it mean that I should add a second Raw representation, or should I just revert it to the 0xXXXX_XXXX_XXXX_XXX format?

Please revert it to the 0xXXXX_XXXX representation. Also, for base16, please bring back the grouping of 4 and underscore separators. I think they make it easier to read really long numbers like ulong.

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