Rockbox

Tasklist

FS#6652 - 'none' keyword in english.lang messes up string order

Attached to Project: Rockbox
Opened by Nils Wallménius (nls) - Monday, 12 February 2007, 19:25 GMT
Last edited by Nils Wallménius (nls) - Tuesday, 24 July 2007, 17:29 GMT
Task Type Bugs
Category Language
Status Closed
Assigned To Daniel Stenberg (bagder)
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

If the keyword 'none' is used in english.lang to remove a string from the
binary .lng file for a specific target, the order of the strings in the
.lng file is not the same as the built in strings, which leads to wrong
strings appearing.

For example, using the following phrase:

<phrase>
id: LANG_CONFIRM_SHUTDOWN
desc: in shutdown screen
user:
<source>
recorder: "Press OFF to shut down"
*: none
</source>
<dest>
recorder: "Press OFF to shut down"
*: none
</dest>
<voice>
recorder: ""
*: none
</voice>
</phrase>

Causes strings in english.lng to be wrong for all targets except the recorder.
The built in strings however are correct.

I have done some investigation, but my understanding of the language build system
and perl is quite limited so, I haven't found the cause.

What I have found is that an english.lng generated for h300 with the above phrase
in english.lang is _exactly_ the same size as one generated with the phrase removed from
english.lang. However they are not identical, diffing the two files tells me they are different.

One further observation I have made is that the strings following the one in the example are the ones that
are in the wrong place and it appears that each of the affected strings are in the place of the one that
comes after it in english.lang.
This task depends upon

View Dependency Graph

This task blocks these from closing
 FS#6574 - Lang v2 cleanup 
Closed by  Nils Wallménius (nls)
Tuesday, 24 July 2007, 17:29 GMT
Reason for closing:  Fixed
Additional comments about closing:  The "lang v2 cleanup" patch now includes a fix by Dave Chapman so that buildzip.pl calls genlang with the features list

Bagder committed a fix for the -u option in genlang on June 26
Comment by Daniel Stenberg (bagder) - Thursday, 15 February 2007, 16:11 GMT
The problem seems to be that 'genlang' takes the last pattern that matches for each phrase, so using "*:" before "recorder:" will work.

This begs the question: should we change this in genlang or adjust the lang to work like this?
Comment by Nils Wallménius (nls) - Thursday, 15 February 2007, 17:11 GMT
Thanks for investigating Bagder. However when switching around
the strings so that the "*:" is before "recorder:" I get the exact
same behaviour --> The strings appear in the wrong places.

I attached a screenshot of the main menu, it looks the same both
with the above example and with this:

<phrase>
id: LANG_CONFIRM_SHUTDOWN
desc: in shutdown screen
user:
<source>
*: none
recorder: "Press OFF to shut down"
</source>
<dest>
*: none
recorder: "Press OFF to shut down"
</dest>
<voice>
*: none
recorder: ""
</voice>
</phrase>

Comment by Daniel Stenberg (bagder) - Thursday, 15 February 2007, 21:03 GMT
Well, I must admit I didn't do a full build with my work but I'm trying to sort out the issue on single file level first. I've attached my "test.lang" that I used to repeat the problem and work with this issue.

With the test.lang file, I generate C and H files using a command line like:

(for h120)
$ genlang -e=test.lang -p=testlang -t=h120 test.lang

(for recorder)
$ genlang -e=test.lang -p=testlang -t=recorder test.lang

Both these command lines generate testlang.[ch] and they seem correct for me when I use the order as in the attached file.

Ehum. Are you saying that the built-in version works fine for you but that the .lng file is broken?

Doing the same language but generating a .lng file is done like this:

(for h120)
$ genlang -e=test.lang -b=testlang.lng -t=h120 test.lang

(for recorder)
$ genlang -e=test.lang -b=testlang.lng -t=recorder test.lang

... and they seem to differ for that shutdown message.

Can you see any obvious errors when using the tool like this? I'm not saying there's no error, I'm just trying to understand where it appears...
Comment by Nils Wallménius (nls) - Friday, 16 February 2007, 17:39 GMT
Bagder, thanks again for looking in to this! :-)
Yes, my problem is that the built in strings are correct but in the .lng files they are not.
(I'm sorry if I was unclear)
I now made a test with your test file and it has the same strange behaviour that I saw in
englis.lang with the example above.

$ genlang -e=test.lang -b=testlang.lng -t=h120 test.lang
with your test.lang produces a 21 byte testlang.lng

when I completely removed the phrase for LANG_CONFIRM_SHUTDOWN and ran
$ genlang -e=test.lang -b=testlang2.lng -t=h120 test.lang
command I once again got a 21 byte .lng file.

"$ diff testlang.lng testlang2.lng
Binary files testlang.lng and testlang2.lng differ"

I opened them in a hexeditor and found the one place they are different.
The byte before "Maybe" is "03" in testlang.lng but "02" in testlang2.lng
I assume this is the string ID number which makes the numbers in the testlang.lng
file go like this: 00Yes..01No..03Maybe

and in testlang2.lng (where the recorder string was removed from the .lang)
00Yes..01No..02Maybe

So it seems the ID numbers are wrong.
Comment by Daniel Stenberg (bagder) - Friday, 16 February 2007, 21:58 GMT
Thanks for your details.

The culprit of this problem is the code starting at genlang line 175 that deals with reading an english .lang file to get the english order to use for output. This code does not deal with 'none' so it will count all IDs present in the file. The same code is not used when the C and H files are generated so for this case, 'none' will completely removed phrases marked as 'none'.

The only sensible fix here is to fix the code that gets the english order to properly check for and acknowledge the 'none' keyword. It isn't however a trivial fix but will take some further parsing and calling the pattern match function etc. I'll try to get some time for this soon if nonody else does it.
Comment by Daniel Stenberg (bagder) - Sunday, 20 May 2007, 13:21 GMT
A fix for this is now in SVN. Can you see if things now work for you?
Comment by Nils Wallménius (nls) - Sunday, 20 May 2007, 20:56 GMT
Genlang now generates a correct test.lng from the test.lang file posted above so it does indeed look good.
I hope to get the langv2 cleanup in sync tomorrow and test with that too.
Comment by Nils Wallménius (nls) - Monday, 21 May 2007, 21:29 GMT
Good news, this does indeed fix the problem above :-) (although with a small quirk, it only
works for 'none' if I write 'NONE' it behaves as before, with gaps in the id numbers, took
me a while to find out but now it doesn't matter much)

Bad news, another problem surfaced that creates almost the same effect :-(
buildzip.pl does not send the features from features.txt to genlang when building the
.lng files it only sends the regular target name, so any strings that only have a feature
match and no match to the target name will be excluded...

If I find some time I will try to find a solution but I have so far in my life never written
a single line of pearl, so it will probably take a while...

Anyway thanks for the fix, one step cloesr to getting the cleanup comitted.
Comment by Daniel Stenberg (bagder) - Tuesday, 22 May 2007, 07:18 GMT
Ah, yes. 1) The NONE issue can probably be fixed by switching to proper case insensitive checking all over. 2) The buildzip problem will need a fix similar to what is used in the build makefile for the features.

I'll try to have a go at these soon.
Comment by Magnus Holmgren (learman) - Sunday, 03 June 2007, 13:17 GMT
Unfortunately, genlang in current svn doesn't handle -u well at all at the moment. Basically, -e doesn't read anything. Using 'dest' to parse the string in this case is incorrect, as a target hasn't been specified. Specifying a target does make things work better.

Also, there seems to be some debug output left.

Loading...