https://bugs.winehq.org/show_bug.cgi?id=42563
Bug ID: 42563 Summary: regedit: minor enhancements/corrections to REG_BINARY edit dialog Product: Wine Version: 2.2 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: minor Priority: P2 Component: programs Assignee: wine-bugs@winehq.org Reporter: galtgendo@o2.pl Distribution: ---
Created attachment 57477 --> https://bugs.winehq.org/attachment.cgi?id=57477 patch implementing the changes
OK, it took me awhile.
As I've written a few weeks ago, due to the display part being unscrollable/unresizable, long values may sometimes not fit into their allocated display space. Also, monospace makes simply much more sense here.
The attached patch modifies the default font to monospace variant and tries to enforce it by using ExtTextOutW.
A possible further enhancement would be to use the proper value for max 0-9A-F width in address block, instead of the same as in the content block.
https://bugs.winehq.org/show_bug.cgi?id=42563
Rafał Mużyło galtgendo@o2.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Hardware|x86 |x86-64
https://bugs.winehq.org/show_bug.cgi?id=42563
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, patch
--- Comment #1 from Austin English austinenglish@gmail.com --- Patches should be sent to wine-patches@winehq.org, see https://wiki.winehq.org/Submitting_Patches
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #2 from Rafał Mużyło galtgendo@o2.pl --- (In reply to Austin English from comment #1)
:sigh:
It's more of a draft of a patch, than a real one. I consider it good enough for me, but not quite for wine. It needs some polish and a decent review from someone who actually knows Windows API, instead of cargo-culting pieces of wine code based on MSDN pages.
Also, making that editor window horizontally scrollable would improve it it significantly.
Even that indent fix for the final line in this patch would be just a style fix in comparison.
https://bugs.winehq.org/show_bug.cgi?id=42563
Rafał Mużyło galtgendo@o2.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://www.winehq.org/pipe | |rmail/wine-devel/2017-Janua | |ry/115910.html
https://bugs.winehq.org/show_bug.cgi?id=42563
Nikolay Sivov bunglehead@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEEDINFO Ever confirmed|0 |1
--- Comment #3 from Nikolay Sivov bunglehead@gmail.com --- Hi, Rafał.
Could you describe what is currently broken that your patch fixes, and how to reproduce it?
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #4 from Rafał Mużyło galtgendo@o2.pl --- Well, technically, much of it is an aesthetic problem.
The changes of the patch:
1. IMHO, it makes no sense for the hexview component to use anything but monospaced font or its closest possible emulation (so, ExtTextOutW)
2. the last line of the hexview, if it's shorter than the preceding lines, isn't properly justified (that is, hex part doesn't start in the same column as previous lines)
3. TBH, I can't really recall anymore what exactly does SetCaretPos change does (too much time since the patch was written), but I *think* it was something about caret behavior when the line was is long enough to not fit into the unscrollable widget...it's likely something obvious...
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #5 from Rafał Mużyło galtgendo@o2.pl --- Reading the patch more, I'm not quite sure anymore, where did I get that '32' from, but the purpose of those extra three spaces almost certainly is 'right margin'.
The way I recall (and the mail strongly suggests) the exact values were my 'mark 1 eyeball' guesstimates.
On not-quite related note: problems three and four from that mail still exist and are much more of a pain than this particular issue (problem two either got solved either on my side or yours or became unnoticeable since).
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #6 from Gijs Vermeulen gijsvrm@gmail.com --- According to: https://www.winehq.org/pipermail/wine-devel/2018-March/123968.html this should be fixed with: https://source.winehq.org/git/wine.git/commit/31e41a942e37db0d37183e8b352541...
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #7 from Rafał Mużyło galtgendo@o2.pl --- (In reply to Gijs Vermeulen from comment #6)
According to: https://www.winehq.org/pipermail/wine-devel/2018-March/123968.html this should be fixed with: https://source.winehq.org/git/wine.git/commit/ 31e41a942e37db0d37183e8b35254177dcdcbbd2
:roll: according to me, it damn well isn't.
I've already spoke with zf, shortly after 3.4 release about his patch. While the offset is a neat addition, ANSI_FIXED_FONT is actually monospace if fontconfig returns such font for Courier; if instead it returns - for example - 'Liberation Mono', the above noted things are still broken.
https://bugs.winehq.org/show_bug.cgi?id=42563
Rafał Mużyło galtgendo@o2.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #57477|0 |1 is obsolete| |
--- Comment #8 from Rafał Mużyło galtgendo@o2.pl --- Created attachment 60809 --> https://bugs.winehq.org/attachment.cgi?id=60809 updated patch
This is an updated version of the patch, though for some reason it works less well now (once the scrollbar reaches bottom position, there's still invisible content below...likely, I need to look at that function that updates the scrollbar).
https://bugs.winehq.org/show_bug.cgi?id=42563
Rafał Mużyło galtgendo@o2.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #60809|0 |1 is obsolete| |
--- Comment #9 from Rafał Mużyło galtgendo@o2.pl --- Created attachment 60810 --> https://bugs.winehq.org/attachment.cgi?id=60810 re-updated patch
...or perhaps I just need to look closer when manually updating a patch - untested, but pretty much same content, except for the accidental side effect the previous one had.
https://bugs.winehq.org/show_bug.cgi?id=42563
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12@gmail.com
--- Comment #10 from Zebediah Figura z.figura12@gmail.com --- This patch distinctly bothers me. The width of the window should probably be dynamically altered to accomodate different font widths (and DPI?), but using ExtTextOut() should be unnecessary if the font returned is indeed monospace.
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #11 from Rafał Mużyło galtgendo@o2.pl --- (In reply to Zebediah Figura from comment #10)
This patch distinctly bothers me. The width of the window should probably be dynamically altered to accomodate different font widths (and DPI?), but using ExtTextOut() should be unnecessary if the font returned is indeed monospace.
I've never said this patch was a correct solution, just a semi-working one, under my limited understanding of winapi (so, those extra spaces at the end, instead of a horizontal scrollbar). Yet, as for the later part: - IIRC, the idea of using ExtTextOut was one I've picked from wine source - under fontconfig, only the program can guarantee the font picked is actually *printed* monospace, the font itself is a system/user preference
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #12 from Zebediah Figura z.figura12@gmail.com --- (In reply to Rafał Mużyło from comment #11)
I've never said this patch was a correct solution, just a semi-working one, under my limited understanding of winapi (so, those extra spaces at the end, instead of a horizontal scrollbar).
Yes, I'll try to fix this at some point. Unless you beat me to it :-)
For the record, I think the correct solution is to fix the width to 8 bytes, and then programmatically set the size upon creation based on that width + a bit for the scrollbar.
Yet, as for the later part:
- IIRC, the idea of using ExtTextOut was one I've picked from wine source
- under fontconfig, only the program can guarantee the font picked is
actually *printed* monospace, the font itself is a system/user preference
It seems to me that if the user sets the default monospace font to something that is not monospace, then it's not our responsibility to enforce otherwise...
https://bugs.winehq.org/show_bug.cgi?id=42563
--- Comment #13 from Rafał Mużyło galtgendo@o2.pl --- (In reply to Zebediah Figura from comment #12)
(In reply to Rafał Mużyło from comment #11)
Yet, as for the later part:
- IIRC, the idea of using ExtTextOut was one I've picked from wine source
- under fontconfig, only the program can guarantee the font picked is
actually *printed* monospace, the font itself is a system/user preference
It seems to me that if the user sets the default monospace font to something that is not monospace, then it's not our responsibility to enforce otherwise...
Well, Liberation Mono is sufficiently monospace for native (that is Linux) programs, I didn't dig deep enough into wine to figure out why it isn't for wine.
Also, some of wine's font handling is plain weird - see two of my bugs, for example: - bug 41687: my hackish fix is made in place which should affect only unaliased bitmap fonts, yet it fixes the opposite cases - bug 42606: for some reason, wrong codepage is picked for the font, leading to mojibake
OK, only the first one might be tangentially relevant, I'm just (yet again) fishing for hints.
Still, per wiki page: "Liberation Mono is styled closer to Liberation Sans than Monotype's Courier New, though its metrics match with Courier New.".
https://bugs.winehq.org/show_bug.cgi?id=42563
tokktokk fdsfgs@krutt.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fdsfgs@krutt.org
https://bugs.winehq.org/show_bug.cgi?id=42563
Linards linards.liepins@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |linards.liepins@gmail.com
--- Comment #14 from Linards linards.liepins@gmail.com --- (In reply to Rafał Mużyło from comment #13)
(In reply to Zebediah Figura from comment #12)
(In reply to Rafał Mużyło from comment #11)
Yet, as for the later part:
- IIRC, the idea of using ExtTextOut was one I've picked from wine source
- under fontconfig, only the program can guarantee the font picked is
actually *printed* monospace, the font itself is a system/user preference
It seems to me that if the user sets the default monospace font to something that is not monospace, then it's not our responsibility to enforce otherwise...
Well, Liberation Mono is sufficiently monospace for native (that is Linux) programs, I didn't dig deep enough into wine to figure out why it isn't for wine.
Also, some of wine's font handling is plain weird - see two of my bugs, for example:
- bug 41687: my hackish fix is made in place which should affect only
unaliased bitmap fonts, yet it fixes the opposite cases
- bug 42606: for some reason, wrong codepage is picked for the font, leading
to mojibake
OK, only the first one might be tangentially relevant, I'm just (yet again) fishing for hints.
Still, per wiki page: "Liberation Mono is styled closer to Liberation Sans than Monotype's Courier New, though its metrics match with Courier New.".
Is this issue still present?