collective / i18ndude

i18ndude performs various tasks related to ZPT's, Python Scripts and i18n.
https://pypi.org/project/i18ndude
4 stars 9 forks source link

find untranslated does not find strings #16

Closed gforcada closed 9 years ago

gforcada commented 9 years ago

I have a super simple viewlet that is basically just a link:

<a
  href="http://www.freitag.de"
  tal:condition="view/display">Go to der Freitag</a>

Running i18ndude find-untranslated src/ reports that the .pt file is fine and thus is not missing any string to be marked as translatable.

If I change that to:

<html>
<head><title>something</title></head>
<body>
<a
  href="http://www.freitag.de"
   tal:condition="view/display">Go to der Freitag</a>
</body>
</html>

Then it does report about a missing string to be marked as translatable.

I see two solutions here:

My suggestion (and hopefully PR later) would be the first, as there is only benefits of having proper HTML on a template (better highlighting on your editor of choice, i18ndude will not complain and will find untranslated strings ...).

Ideas? opinions?

mauritsvanrees commented 9 years ago

In common.py we have the prepare_xml function. This is called and translates your input to:

<?xml version="1.0" encoding="utf-8"?><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html xmlns:tal="tal" xmlns:i18n="i18n" xmlns:metal="metal"><a
  href="http://www.freitag.de"
  tal:condition="view/display">Go to der Freitag</a>
</html>

I see some bad code in common.py that has been there since ten years... It can definitely use some improvements. I started a bit on branch maurits-fix-prepare-xml. Feel free to work on that branch if you have more ideas.

Wait, when I try your second snippet, i18ndude indeed complains about a missing translation, but it is not the Go to der Freitag that it complains about: it is the something from the title that you added! Do you see that too? When I remove the tal:condition, it rightly complains about the missing translation. But this also works in the first snippet...

I tried it in a template from standard Plone, with all the boiler plate necessary, and I had the same problem.

Okay, this is an interesting one. Create a file without any boiler plate and just these three lines:

<span tal:condition="nothing">Line 1</span>
<tal:block tal:condition="nothing">Line 2</tal:block>
<tal:block condition="nothing">Line 3</tal:block>

i18ndude find-untranslated should complain about all three lines, right? It only complains about the second line... Something is definitely fishy here.

mauritsvanrees commented 9 years ago

There may be some special handling for the tal:condition="nothing" case, but this file gives the same result:

<span tal:condition="view/foo">Line 1</span>
<tal:block tal:condition="view/foo">Line 2</tal:block>
<tal:block condition="view/foo">Line 3</tal:block>
mauritsvanrees commented 9 years ago

Okay, that was actually easy to fix once I found the cause. What boggles me is that this part has not been changed in ten years, so this error has been there all along...

Can you check the branch and see if it helps?

gforcada commented 9 years ago

Will do, thanks for the investigation!

ale-rt commented 9 years ago

Should this be closed because of 8a0c3f7?

mauritsvanrees commented 9 years ago

I think so. @gforcada I have released 3.4.5 with a fix. Please reopen if there is still a problem

gforcada commented 9 years ago

@mauritsvanrees will test and report back, thanks a lot! (also to @ale-rt !)