Warning for slashes in domains #76
Conversation
|
If the domain has a / in it, we can say in the error message that slashes in domain names are not supported. For this case, we may not even need to show the available domains. I don't remember why we would want to show the known domains anymore, hahaha (sorry!). |
| def warn_if_domain_has_slashes(backend, domain) do | ||
| if String.contains?(domain, "/") do | ||
| IO.puts :stderr, """ | ||
| The domain #{inspect domain} was not compiled. The #{inspect backend} |
whatyouhide
force-pushed
the
warning-for-slashes-in-domains
branch
from
2058759
to
4d9bf4a
Compare
|
@josevalim makes sense :) I squashed the second commit, now we warn when the domain is unknown (and we show the available domains) and we warn that |
whatyouhide
force-pushed
the
warning-for-slashes-in-domains
branch
from
4d9bf4a
to
b3da17d
Compare
| cond do | ||
| String.contains?(domain, "/") -> | ||
| IO.puts :stderr, "warning: slashes in domains are not supported: #{inspect domain}" | ||
| not domain in known -> |
There was a problem hiding this comment.
Discussion point: should we really warn here? Should we ask folks to create empty .po files in their locales then? I would say we shouldn't warn here (we can still keep known_domains around, I would just remove the warning. If we warn, we will break the workflow where I can just add translations, specially in english, without caring about the .po / .pot files.
There was a problem hiding this comment.
I'm 👍 for not having this warning or make it configurable, e.g. allow_missing: false. 
There was a problem hiding this comment.
Honestly, I don't want to warn on missing domains. It's a little bit of a mess, we have to capture so much IO in the tests and @josevalim makes a great point (you'll be warned everytime until you run mix gettext.extract).
|
Sorry I was not super clear in my previous comment(s). My point is simply that we can keep known_domains around but I don't think we would use it for anything right now. I think the warning will cause more confusion than help. :) |
|
And to be super clear: the warning on slashes is still super useful. |
| domains known by the backend. | ||
|
|
||
| This function is called by `lgettext` and `lngettext`. | ||
| """ |
There was a problem hiding this comment.
Maybe it should be @doc false? Do we expect end users will use this function.
There was a problem hiding this comment.
The whole module is @moduledoc false, all other functions are documented with @doc.
|
Ok, we're superclear now: we'll keep the known locales around (cause it's easy and it can be useful) and we'll only warn for domains with |
You mean known domains, right? |
|
Feel free to remove known_domains. but it doesn't hurt to keep it as well.
|
whatyouhide
force-pushed
the
warning-for-slashes-in-domains
branch
from
b3da17d
to
0947a3c
Compare
|
Ok @josevalim and @lexmag, I decided to remove I also fixed the warnings: now only domains with slashes in them emit warnings when used. Ready to merge? |
|
|
Warning for slashes in domains
|
Hi there! I might be missing something, but how do you treat nested domains then? For example, |
|
@vborodkin is this something GNU gettext supports? |
|
@whatyouhide I don't know. I never used gettext until I started using Phoenix, which is recently. Is there anything I can do to help and clarify this? Although, it seems fairly normal to namespace domains in larger applications. I can use a underscore for the time being. |
|
GNU gettext does support EDIT: In fact it seems to handle about any characters I throw at it... |
Implements the ideas discussed in #75.