FtpServer icon indicating copy to clipboard operation
FtpServer copied to clipboard

LANG catalog loader - unnecessary NGettext dependency and debug log messages

Open Kissaki opened this issue 3 years ago • 1 comments

It looks like IFtpCatalogLoader/DefaultFtpCatalogLoader implements FTP i18n (RFC 2640) with a dependency on NGettext, claims to only support en, but then does not provide the en texts?

This means that the Abstractions package depends on the NGettext package, and that NGettext produces debug output messages on every FTP command response (with text).

Example of the debug output - even when default LANG en remains active (no LANG command has been sent):

NGettext: Translation not found for message id "User {0} logged in, needs password".
NGettext: Translation not found for message id "Password ok, FTP server ready".
NGettext: Translation not found for message id "Command okay.".
NGettext: Translation not found for message id "Protection buffer size set to {0}.".

Given that nothing seems to be implemented, this seems unnecessary.

Currently, IFtpCatalogLoader references the NGettext ICatalog type. The default terminology and interface seem to indicate that there is intention of supporting other solutions without NGettext. For such cases, depending on an NGettext interface seems inappropriate. It would seem better for FtpServer as a lib to define its own ICatalog equivalent interface, if its method are the desired/intended approach.

With the NGettext interface dependency resolved/dropped, a better default implementation of IFtpCatalogLoader would be to simply return the requested text, and declare it to be en.

Overall these changes would drop the NGettext dependency and reduce unnecessary debug logged messages. In my opinion, this is desired and reasonable.

Could you please give insight into intention and decisions regarding this, as well as intention or acceptance into changes regarding this?

Kissaki avatar Sep 20 '22 08:09 Kissaki

You're right, that the dependency on NGettext isn't really needed. I used it to support gettext-style translation support out of the box, but it seems that it's currently not useful/necessary in its current state.

fubar-coder avatar Sep 21 '22 05:09 fubar-coder

Looks like the NGettext dependency version upgrade in d55e5ecf8c2d2acc251f9e1d90942c159395f8a9 (commit title mismatch) at least resolved/evades the trace log noise. NGettext changed when they emit them in https://github.com/VitaliiTsilnyk/NGettext/commit/1be6dd42be2626a916c2e143d46b540e22612cc0 - no longer for release builds.

This change is not in a FtpServer release yet though.

Kissaki avatar Dec 22 '23 14:12 Kissaki