[Bug 20977] Bad hlp table rendering

[wine-bugs at winehq.org]

https://bugs.winehq.org/show_bug.cgi?id=20977

--- Comment #23 from Sebastian Lackner <sebastian at fds-team.de> ---
(In reply to Jice from comment #13)
> Created attachment 53752 [details]
> 0001-winhlp32-fix-table-formatting.patch

Just had a look at your patches. As a general remark, the subject line "Fix
table formatting" is not really very meaningful and only fits for the last
patch. For the other patches, it would be better to describe the real changes,
and avoid the extra description in the next line. Some more detailed comments:

1/6 - The idea is fine, but I don't think you catched all occurances of those
constants. HLPFILE_SkipParagraph also compares against hardcoded 0x20 and 0x23
values for example.

2/6 - There is never a need to TRACE the function name itself, it will
automatically be added by Wine in front of your message. Strings should be
passed through debugstr_a() to ensure they are valid before passing them to
TRACE.

3/6 - Should be fine.

4/6 - You are adding commented-out code here, which should be avoided. Those
parts should probably go into to a later patch, when it is really needed. The
detection based on face_offset also looks a bit suspicious.

5/6 - I'm not sure about the rounderr, otherwise it should be fine.

6/6 - There are still various formatting changes in there, I would suggest to
remove them or move them to previous patches. Since this is the most critical
patch its good to keep the number of unrelated changes as small as possible.

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.