gettext-rs icon indicating copy to clipboard operation
gettext-rs copied to clipboard

Symlinks and paths ending with /locale not supported by TextDomain

Open sophie-h opened this issue 4 years ago • 6 comments

I tried to use TextDomain with flatpak and two issues prevent me from using it:

  1. The locale dir passed to the buildsystem ends with /locale. Would be nice if this is supported by TextDomain::prepend as well. (Easy to work around.)
  2. Flatpak or Flathub uses symlinks of the form /app/share/locale/es -> ... But TextDomain::init() does not follow symlinks inside locale for the checks. This makes it basically unusable for this case.

sophie-h avatar May 20 '21 13:05 sophie-h

Hi!

The locale dir passed to the buildsystem ends with /locale. Would be nice if this is supported by TextDomain::prepend as well.

I'm not sure I understand correctly. Do you mean that TextDomain shouldn't append its own /locale to the path that's given to it?

The second point makes total sense. I'll gladly merge a PR that makes TextDomain follow symlinks, and make a minor release afterwards.

Minoru avatar May 20 '21 17:05 Minoru

Sorry, didn't really explained 1. properly. I'm getting passed /app/share/locale from the build system. But TextDomain::prepend expects only /app/share without locale. Since passing it with the locale part seems to be the default for gettext, it would be nice if I could also pass it with locale to TextDomain such, that I don't have to strip locale manually with .partent() etc. before passing is to TextDomain::prepend.

Looking forward to the fix! Thank you!

sophie-h avatar May 20 '21 18:05 sophie-h

Ah, got it. Yeah, I think it makes sense for TextDomain to do what gettext does, and not append anything to paths. That's a breaking change and will require a major release, but that's fine.

By "I'll gladly merge a PR" I meant that I won't work on this myself, at least not right now, but I'm available to review and merge a PR if one comes along. In other words, it's an invitation for you to fix this :)

Minoru avatar May 20 '21 18:05 Minoru

Ah sorry, somehow I misread you. Will put it on my list.

Maybe it makes sense to rename the functions to make the 1. change also breaking for the compiler. They are not too self explaining right now either.

sophie-h avatar May 20 '21 19:05 sophie-h

Yeah, I didn't like them either, but I never came up with better ones. I thought of something like search_before_system() and search_after_system(), but that's pretty long and still not entirely clear. If you have ideas on that front, I'm all ears!

Minoru avatar May 20 '21 19:05 Minoru

Re-opening because the first point still isn't addressed.

Minoru avatar Aug 30 '24 19:08 Minoru