leonelquinteros / gotext

Go (Golang) GNU gettext utilities package
Other
434 stars 58 forks source link

`Get("")` is returning all headers #74

Closed taozhou-glean closed 1 year ago

taozhou-glean commented 1 year ago

Please describe your issue

due to existing code structure, sometimes we have to call Get() with vars, and I just noticed that the return is the entire header when its empty string, seems expected given the header is always formatted as:

msgid ""
msgstr ""
"POT-Creation-Date: 2023-04-29 14:35-0800\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Language: en\n"

but also a bit surprise as its not expected outcome, so we have to check str before calling Get which is a bit strange.

Is this a bug, an improvement, a proposal or something else? Describe it.

Function wise, its not technically a bug, but from user perspective, it is

What's the expected behaviour, the current behaviour and the steps to reproduce it?

I would say we should just return "" in this case, it does not really make sense to have empty string id in the message file.

Comments

leonelquinteros commented 1 year ago

I'd say, let's figure out what gettext does with an empty string and try to replicate. BUT, not sure on changing current behavior due to backwards compatibility, in case somebody is using Get("") to obtain the headers.

taozhou-glean commented 1 year ago

well, looks like that is indeed expected behavior from gnu gettext: https://www.gnu.org/software/gettext/manual/gettext.html#MO-Files:~:text=as%20the%20empty%20string%20in%20a%20PO%20file%20GNU%20gettext%20is%20usually%20translated%20into%20some%20system%20information%20attached%20to%20that%20particular%20MO%20file%2C%20and%20the%20empty%20string%20necessarily%20becomes%20the%20first%20in%20both%20the%20original%20and%20translated%20tables%2C%20making%20the%20system%20information%20very%20easy%20to%20find.

it's quite inconvenient tho, but I guess most of times, the expected usage here is always inline the message you want to translate instead of passing it as a variable.

taozhou-glean commented 1 year ago

I understand this lib is trying to align with gettext, just wondering if it's a reasonable ask to provide a global config to disable the "" -> meta behavior ? we can still have it false by default so its backward compatible

leonelquinteros commented 1 year ago

I'll reject this one as I prefer to stick to gettext compatibility rather than general usage. There may be many edge cases like yours for implementations of this package, we can't always cover every case, but provide tools for them to adapt. You should be able to wrap Get calls, or have general filters in place to safely use empty vars with this package.

You can also remove the headers from your (deployed) translation files, so Get("") returns "", as it seems you don't use these headers in your code at all.