davetcc / tcMenu

Menu library for Arduino, mbed and ESP with designer UI and remote control capabilities.
https://www.thecoderscorner.com/products/arduino-libraries/tc-menu/
Apache License 2.0
272 stars 25 forks source link

[i18n] Generated locale sources are incomplete, resulting in build errors #345

Closed vzahradnik closed 1 year ago

vzahradnik commented 1 year ago

Describe the bug After enabling experimental locale support for the first time and generating sources, the resulting code fails to build.

Commerical or Personal use Commercial use.

To Reproduce

Notes

image

image

Expected behavior Generated sources successfully compile. I don't know what is the expected behavior - either all the strings should be properly extracted to the default i18n language file or the generated code should not expect strings if we didn't modify a menu item.

davetcc commented 1 year ago

So as usual, it was a few really minor issues causing this, I think it is all fixed now, and I've added a couple of extra unit tests to make sure it stays fixed. The issues were

  1. There was an error in the default implementation that got the string from the resource, it did not account for the possibility of a unit called "%" or it being blank.
  2. There was a bug in the loading and saving of the bundles (as this didn't use the resource bundle serialization code as we needed more control over things. It couldn't handle empty values.
vzahradnik commented 1 year ago

Cool, I'll test your changes.

vzahradnik commented 1 year ago

I tested with your latest changes and the issue still persists. Resource bundles are empty and yet the generated cpp code expects units to be defined as external strings.

davetcc commented 1 year ago

Did you edit the item once, as you'd be in a broken state until you did that. IE a state that the system should never have got into (where it wrote out a localized menu unit, but not the value.

You may need to edit the item once, to get the actual entry there first.

davetcc commented 1 year ago

I've been fixing bugs in the whole localization process this morning, more to come soon to get enums working properly

davetcc commented 1 year ago

I think we need a fix project option, it is very likely that there will be some point in time where there is either a crash of designer, or some kind of malfunction that would leave entries in the project emf file that were not in the properties. Maybe a simple CLI command, run from the directory with the faulting file like tcmenu repair that just puts blank entries or highlights any missing entries.

vzahradnik commented 1 year ago

I deliberately didn't edit any item. What I did:

I wanted to make sure that the generated sources compile all the time.

davetcc commented 1 year ago

Fully understand now, will test this next.

vzahradnik commented 1 year ago

I think we need a fix project option, it is very likely that there will be some point in time where there is either a crash of designer, or some kind of malfunction that would leave entries in the project emf file that were not in the properties. Maybe a simple CLI command, run from the directory with the faulting file like tcmenu repair that just puts blank entries or highlights any missing entries.

Sure, why not? In my situation, this is however not the case. For testing this feature I use project file prior enabling i18n support.

davetcc commented 1 year ago

With the latest code, I just converted tcMenu/examples/esp/esp32s2Saolato be an i18n project, it compiled and ran first time after I added French to enable the locale support. I did not touch any of the menu items whatsoever. I was able to build the project. As far as I can see, the enum items are tip-top.

There were some bugs around enum item conversions that I fixed earlier on. If you can recreate this still, could you provide me the project file (before conversion) and I'll try it myself.

vzahradnik commented 1 year ago

It still doesn't work, so here is the project file. https://gist.github.com/vzahradnik/8ddb94ffc710a0212860128c6812888a

davetcc commented 1 year ago

Got it now, it is the item that has a % followed by more text. during the conversion to i18n designer needs to escape any '%' symbols. Should be a fairly quick fix. I would suggest backslash% or %% as the right method. Which do you think looks better?

vzahradnik commented 1 year ago

I'd say backslash %.

davetcc commented 1 year ago

I think this should be fixed now, there is a step during i18n initialization that makes sure all %'s at the start position are escaped.

vzahradnik commented 1 year ago

Yes, I tested that and found no issue. My project compiles fine. You can probably close the issue now.