|
From: <apn...@ya...> - 2025-12-31 13:44:45
|
Jan,
Thanks for taking the time for reviewing. It is appreciated.
Responses to your concerns are below, but in conclusion I have decided to remove the error related C API from the TIP. As I had already mentioned in the TIP, they were completely ancillary to the main purpose of the TIP and I don't want to delay the TIP on their behalf. In hindsight I think it was a mistake to include them in this TIP in the first place. I was just lazy to create yet another TIP and subject folks to yet another vote.
With respect to the locale issue you raised, I thought about it at the time and could see the rationale for both localized and English error messages. In the end, I decided to stick with the existing localized messages as that is what (as you stated) is done in 8.6 and 9.0.
With respect to your comment regarding msgcat being the standard method to localize messages, while that may be the case for applications, I think it is debatable if that is appropriate for system generated error messages. Are users supposed to create message catalogs for the gadzillion possible Windows error messages? So the point is debatable at the very least.
With respect to the locale parameter, if we go down that route, it is worth thinking whether the parameter should be a locale string or a LCID. Having written several Windows-specific extensions, I find LCID's both more convenient and comprehensive but that is offset by portability to non-Windows platforms. But keep in mind the functions are very Windows specific to begin with. So again, it is something that would need discussion.
All that leads to removal of the API from the TIP. Long path support does not need any of that as the file system code always translates errors to POSIX errors (losing information in the process but that is not something this TIP fixes anyways) and deals with them accordingly.
Regarding testing on macOS and Linux, I do that as a rule via Github CI, definitely before merging to any of the major branches, but even on private branches. Perhaps I missed that in this case.
/Ashok
-----Original Message-----
From: Jan Nijtmans <jan...@gm...>
Sent: Wednesday, December 31, 2025 3:02 AM
To: apn...@ya...
Cc: tcl...@li...
Subject: Re: [TCLCORE] TIP 744 - long path support for Windows - ready for review
Op za 27 dec 2025 om 15:02 schreef apnmbx-public--- via Tcl-Core
<tcl...@li...>:
>
> TIP 744: Support for long paths on Windows is ready for review.
Well, there's one thing I don't like about the TIP: The error-messages
generated are
always in the current locale, not in en_US. And there is no
possibility to change that.
In my view it was always a mistake that the registry extension
produced localized
error-messages, unlike any other extension. This can be seen by the test-suite:
if the locale is not 'en', many registry testcases are skipped. The
standard 'Tcl'
way is using msgcat to translate messages to other languages for display.
The minimum which should happen is add a "locale" parameter to the functions.
The default should be "en_US" (or "C", if you want, that's actually the same).
I made a simple start in the "tip-744-with-locale" branch, still unfinished
but you get the idea. The function LocaleNameToLCID() can be used to
translate from locale name to LCID.
The second thing which is wrong iIMHO is the handling of the
messageDll parameter. It should be in UTF-8 (like all other parameters),
but it is input directly into LoadLibraryExA(). If a full path is used, which
contains non-ASCII characters this won't work. Also, I don't think it
works with a dll coming from a VFS (I don't see a test-case for that,
but since LoadLibraryExA() doesn't know about the VFS, I'm
sure it won't work).
Finally, the errorCode parameter of Tcl_WinRaiseError() and the messageId
parameter of Tcl_WinAppendMessageFromModule() are actually the
same thing. Why not call them 'messageId' in both functions? That's
how Microsoft names the third FormatMessageW() parameter,
which is also the same thing.
It also would be nice when the UNIX and MacOS builds still work, and
everything compiles without warnings. I committed fixes for that in
the "tip-744" branch, that's less work for me than discussing it here.
Conclusion: still work to be done .... A YES vote for the TIP as it is now
expresses an OK for extensions to output localized error-messages.
I don't think that's OK, it should not be encouraged.
Regards,
Jan Nijtmans
|