MemoryView: Prevent non-ascii characters #5294
Conversation
sepalani
force-pushed
the
mem-view-ascii
branch
2 times, most recently
from
1cc93b4
to
36cda76
Compare
|
The implementation looks good assuming we indeed only want ASCII characters to be visible in the memory view. Most hex editors and similar software don't do that, though. They typically use an encoding like Windows-1252 for interpreting the data, since that makes more characters visible. Does anyone else have an opinion on this? |
|
I'd still opt for something similar to this. |
| str += static_cast<char>(byte); | ||
| char byte = static_cast<char>(mem_data >> (24 - i * 8) & 0xFF); | ||
| if (std::isprint(byte, std::locale::classic())) | ||
| str += byte; | ||
| else | ||
| str += ' '; |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@JosJuice Most "useful" (free/open source) hex editors I know (i.e. MadEdit, WxMedit, wxHexEditor, etc.) don't use per se but let the users choose what encoding they should use. That's probably what we should do even though it isn't as simple as it sounds. @spycrab Might be done in another PR. I also prefer dots over spaces since they are visible. However, I'd rather prefer a monospace-font usage regarding of the character used to replace invalid ASCII characters. |
|
LGTM |
sepalani
force-pushed
the
mem-view-ascii
branch
from
36cda76
to
f8465d0
Compare
This PR prevents non-ascii characters to be printed in the MemoryView when set to view as ASCII. It shouldn't be seen as a replacement for PR #5293 as they address different issues.
Ready to be reviewed & merged.