search

FubarDevelopment / FtpServer

Portable FTP server written in .NET
http://fubardevelopment.github.io/FtpServer/
MIT License
472 stars 161 forks source link

LANG catalog loader - unnecessary NGettext dependency and debug log messages #144

Closed Kissaki closed 8 months ago

Kissaki commented 1 year ago

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?

fubar-coder commented 1 year ago

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.

Kissaki commented 8 months ago

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.