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

hook-merge-pot-in-po does not work #1553

Closed
gilbsgilbs opened this issue Jul 12, 2017 · 5 comments
Closed

hook-merge-pot-in-po does not work #1553

gilbsgilbs opened this issue Jul 12, 2017 · 5 comments
Assignees
Labels
bug Something is broken.
Milestone

Comments

@gilbsgilbs
Copy link
Contributor

gilbsgilbs commented Jul 12, 2017

Yeah, I know, I've written this a few months ago, but it cannot work since it expects to be called multiple times with the .po file as argument. This is not going to happen in post_update scripts.

I don't know what to do exactly. Correct me if I'm wrong, but I don't think we can handle the general in a post_update scripts currently, unfortunately. We may want to remove this hook, but lacking from such a script out of the box obviously makes things a lot more complicated.

@gilbsgilbs
Copy link
Contributor Author

One other possibility would be something like this (I think it will work in my case):

#!/bin/sh
# Post-update hook to merge POT translations into PO.
# Requires gettext.

[ -z "${WL_NEW_BASE}" -o -z "${WL_FILEMASK} ] && exit 0
find . -path "*/${WL_FILEMASK}" -exec msgmerge -U "{}" "${WL_NEW_BASE}" \;
exit $?

What do you think?

@nijel
Copy link
Member

nijel commented Jul 13, 2017

For the long term the solution will be addons doing such things properly in Weblate (see #1353).

As for suggested change, IMHO the * in the find is not necessary, this should work:

find . -path "${WL_FILEMASK}" -exec msgmerge -U "{}" "${WL_NEW_BASE}" \;

@gilbsgilbs
Copy link
Contributor Author

gilbsgilbs commented Jul 13, 2017

@nijel Yep. Definitely can't wait for #1353 which will greatly enhance Weblate continuous translation possibilities.

I've always used -path with */ prefix without wondering, and after a quick test on my machine, it doesn't work when it's missing. It seems to match the path as returned by find command which seems always prefixed by ./ when base path is .. We might replace */ with ./ though 🤔 . */ prefix is really common, but I'm unsure whether it's a best practice or not.

Edit: + we probably want to match files only:

#!/bin/sh
# Post-update hook to merge POT translations into PO.
# Requires gettext.

[ -z "${WL_NEW_BASE}" -o -z "${WL_FILEMASK} ] && exit 0
find . -type f -path "./${WL_FILEMASK}" -exec msgmerge -U "{}" "${WL_NEW_BASE}" \;
exit $?

@nijel
Copy link
Member

nijel commented Jul 14, 2017

Good point with ./, it's indeed needed. Will you please submit PR with the fix?

@gilbsgilbs
Copy link
Contributor Author

@nijel I can submit the PR, but keep I won't be able to test it before monday. So be sure it works :) .

@nijel nijel self-assigned this Jul 14, 2017
@nijel nijel added the bug Something is broken. label Jul 14, 2017
@nijel nijel added this to the 2.16 milestone Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken.
Projects
None yet
Development

No branches or pull requests

2 participants