Expect the substitute, not the lookalike, when the codepage is short - #151
Draft
xroche wants to merge 3 commits into
Draft
Expect the substitute, not the lookalike, when the codepage is short#151xroche wants to merge 3 commits into
xroche wants to merge 3 commits into
Conversation
WideCharToMultiByte approximates a character the target codepage lacks and leaves lpUsedDefaultChar clear while doing it, so the two --selftest blocks that build an ANSI fixture read an approximation as held. On a codepage that best-fits, the catalog check then compared the engine's output against a lookalike: harmless while the engine approximated too, a false FATAL now that it blocks best-fit and writes the codepage default character instead. Both fixtures go through CopyTextWideToACP(), which blocks best-fit as the engine does, so the MBCS gate skips where the codepage genuinely cannot hold the accent and the catalog expectation matches the converter on every codepage. That removes the catalog block's own skip, and adds the case the change is about: U+0100, which cp1252 best-fits to 'A'. The reported line carries a suffix when the codepage has a lookalike to offer, so CI can pin that the case is able to fail on this runner. Needs the engine's best-fit fix (xroche/httrack#1410). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Comparing the engine's output against another WideCharToMultiByte call only proves the two agree; where the codepage has a lookalike to offer, the substitute is now checked against CPINFO.DefaultChar, and the helper must report the text as not held. That check runs only on such a codepage, so its count moves with the suffix already pinned. Also from review: the helper guards destSize first, as its neighbour does, and its name says which way the boolean goes, since it copies on both paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The staged-tree run redirects stdout and stderr to files nobody reads, so a FATAL line naming the failing check is thrown away and the log carries only "--selftest exited 3". Print both when the exit code is non-zero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WideCharToMultiBytesubstitutes a lookalike for a character the target codepage lacks, and leaveslpUsedDefaultCharclear while doing it. So both--selftestblocks that build an ANSI fixture read that approximation as held. On a codepage that best-fits, the catalog check was comparing the engine's output against the lookalike. That was harmless while the engine approximated too. Once xroche/httrack#1410 has it write the codepage default character instead, the same check turns into a false FATAL there.Both fixtures now go through
CopyTextWideToCPExact(), which blocks best-fit the way the engine does. The MBCS gate then skips only where the codepage truly cannot hold the accent, and the catalog expectation agrees with the converter on every codepage. That removes the catalog block's own skip and adds the case this is about: U+0100, which cp1252 best-fits toA. Where the codepage has a lookalike to offer, the substitute is checked againstCPINFO.DefaultCharrather than against a secondWideCharToMultiBytecall, and the reported line grows a(best-fit acp)suffix. CI pins both, so neither the case nor its precondition can go quiet on this runner.Measured on the runner rather than argued: against today's engine both arches fail with
best-fit catalog text gave 'A', expected '?', which is the control saying the new case can fail here at all. Modelled the rest on Linux against Microsoft's bestfit1252 and bestfit932 tables, since none of it is reachable from a build here: 5 checks on cp1252, 4 on cp932 and on a UTF-8 ACP, and removing the flag from both sides at once drops the suffix, which is what the pin is for.Green on engine 9b083e68, which carries that fix:
catalog decoding ok on 5 checks (best-fit acp)on both arches.