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.

Sign up for GitHub

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

Jump to bottom

Introduce 'Endianness' in UI and honor it during group rendering #93

Merged
merged 1 commit into from Mar 8, 2024
Merged

Introduce 'Endianness' in UI and honor it during group rendering #93

merged 1 commit into from Mar 8, 2024

Conversation

martin-fleck-at
Copy link
Contributor

What it does

  • Move Endianness interface into common code
  • Add setting for 'Endianness' in package.json
  • Add option for 'Endianness' in options panel
  • Assume DAP as big endian and revert byte-order for little endian
  • Set 'Little Endian' as default as it may be more common

endianness

Closes #88

How to test

  • Open a Memory Inspector
  • Test the default preference in the Settings
  • Test the individual settings in the memory inspector options
  • See how the byte order changes within a group, not only a word

Review checklist

Reminder for reviewers

@martin-fleck-at
Copy link
Contributor Author

@colin-grant-work Thank you for having a look at this so quickly! I just rebased the branch to resolve the merge conflict (no other changes).

@colin-grant-work
Copy link
Contributor

@martin-fleck-at, looks like we've managed to create new conflicts in the meantime, sorry!

@martin-fleck-at
Copy link
Contributor Author

@colin-grant-work I small price to pay for all that progress we are making! ;-) I rebased the change on the latest master.

@martin-fleck-at
Introduce 'Endianness' in UI and honor it during group rendering
df47e22 
- Move Endianness interface into common code
- Add setting for 'Endianness' in package.json
- Add option for 'Endianness' in options panel
- Assume DAP as big endian and revert byte-order for little endian
- Set 'Little Endian' as default as it may be more common

Closes eclipse-cdt-cloud#88
@martin-fleck-at
Copy link
Contributor Author

Just another rebase :-)

@jreineckearm
Copy link
Contributor

@thegecko , @colin-grant-work , @planger , could one of you please merge before we catch another merge conflict. :-)

jreineckearm
jreineckearm suggested changes Mar 8, 2024
View reviewed changes
Copy link
Contributor

@jreineckearm jreineckearm left a comment •
edited

Choose a reason for hiding this comment

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

Hang on, something's broken with the autofit now, @martin-fleck-at :
It looks like it truncates now cells in the Data column.

Truncated
image

Widened Data column, still truncated but a little less since more space
image

@haydar-metin
Copy link
Contributor

haydar-metin commented Mar 8, 2024 •
edited

@jreineckearm @martin-fleck-at I will have a look at it. My approach to smooth the rendering creates more problems, and it is not worth those headaches.

See #99

@martin-fleck-at
Copy link
Contributor Author

@jreineckearm Do you agree that this PR can then be merged since the issue will be fixed as part of #99?

@thegecko thegecko merged commit 51e738c into eclipse-cdt-cloud:main Mar 8, 2024
5 checks passed
@haydar-metin haydar-metin mentioned this pull request Mar 8, 2024
Merged
1 task
@jreineckearm
Copy link
Contributor

@martin-fleck-at , yes I agree ;-)

@planger planger mentioned this pull request Mar 10, 2024
Open
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreineckearm jreineckearm jreineckearm requested changes

@colin-grant-work colin-grant-work colin-grant-work approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Honor endianess on group level
5 participants
@martin-fleck-at @colin-grant-work @jreineckearm @haydar-metin @thegecko