Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get("") is returning all headers #74

Closed
taozhou-glean opened this issue Apr 29, 2023 · 4 comments
Closed

Get("") is returning all headers #74

taozhou-glean opened this issue Apr 29, 2023 · 4 comments

Comments

@taozhou-glean
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

@taozhou-glean
Copy link
Contributor Author

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
Copy link
Owner

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.

@leonelquinteros leonelquinteros closed this as not planned Won't fix, can't repro, duplicate, stale May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants