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

Missing PO Entry when using String Concatenation (+) or Referencing Text defined in const/Field/Property/Variable #87

Open
BrunoJuchli opened this issue Feb 26, 2024 · 5 comments

Comments

@BrunoJuchli
Copy link
Contributor

BrunoJuchli commented Feb 26, 2024

Repro Cases

String Concatenation +

S["Hey there " +
   "how do you like this?" + 
   "I know it's not recommended"];

String Reference (Field/Property...)

const string MyMessage = "This is my message";

....

S[MyMessage];

Actual Behavior

extractpo runs successfully, but the generated *.po file does not contain any entries for these calls.

Expected (Desired) Behavior

Either one of the following would be fine for me:

  • not support these and communicating it when hitting such a case:
    • crash the app with error message indicating the problem
    • exit the app with an error code, and write to
  • support these so that Hey there how do you like this? I know it's not recommended / This is my message turns up in the PO file, respectively.
    • note that the case of referencing an item would require either
      • a) using the S[...] line as context
      • b) or the one of the field/property, but then would require duplicate detection, because the same field/property might be referenced twice!

Note: for the string +-concatenation I would prefer if it would be supported. The reason is, that this is the only supported way how to break a long single-line string into multiple source code lines. All other approaches introduce new lines.
Also note, that the +-concatenation is compiler optimized so that the compilation actually contains a single string -> in the compiled assembly there is no difference between "a" + "b" and "ab"! Something I've learned today :D

@BrunoJuchli BrunoJuchli changed the title Missing PO Entry when using String Concatenation (+) or Referencing Text defined Otherplace Missing PO Entry when using String Concatenation (+) or Referencing Text defined in Field/Property/const Feb 26, 2024
@BrunoJuchli BrunoJuchli changed the title Missing PO Entry when using String Concatenation (+) or Referencing Text defined in Field/Property/const Missing PO Entry when using String Concatenation (+) or Referencing Text defined in const/Field/Property/Variable Feb 26, 2024
@BrunoJuchli
Copy link
Contributor Author

I have reproduced this with the current version on develop - the behavior is the same as the latest release 1.0.1 = the issue still exists.

@hishamco
Copy link
Member

hishamco commented Mar 3, 2024

I will try to reproduce it

@hishamco
Copy link
Member

@BrunoJuchli sorry for the delay on this, could you please write a unit test for each and push a PR for fixing those?

@BrunoJuchli
Copy link
Contributor Author

BrunoJuchli commented Mar 18, 2024

@hishamco
Thanks for getting back to me and the collaboration!
Unfortunately I don't have enough time at the moment to look into this further.
We're still in the decision phase for how we're dealing with translations - if we do actually decide to use this tool I'll probably come back to it.
I'm fine with you closing the issue, if you want to.

It would be really nice if you could release a new version, though. Since this would fix a few of the issues we experience with the tool, it would go a long way to convince the team.

@hishamco
Copy link
Member

We're still in the decision phase for how we're dealing with translations - if we do actually decide to use this tool I'll probably come back to it.

Please let me know if you need any support on the tool, it's my pleasure that the tool is already used by OC and hopefully other projects

It would be really nice if you could release a new version, though. Since this would fix a few of the issues we experience with the tool, it would go a long way to convince the team.

Ya I will do ASAP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants