OrchardCoreContrib.PoExtractor
OrchardCoreContrib.PoExtractor copied to clipboard
Missing PO Entry when using String Concatenation (`+`) or Referencing Text defined in `const`/Field/Property/Variable
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 messageturns 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!
- a) using the
- note that the case of referencing an item would require either
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
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.
I will try to reproduce it
@BrunoJuchli sorry for the delay on this, could you please write a unit test for each and push a PR for fixing those?
@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.
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
I've figured out the code to extract the string from a string literal concatenation expression like:
["foo" + "bar" + "licious"]
here's the code:
public static bool IsStringLiteralExpression(
ExpressionSyntax expression,
[NotNullWhen(true)] out string? stringLiteralExpression)
{
if(GetLiteralExpressionValue(expression) is { } s)
{
stringLiteralExpression = s;
return true;
}
string concatenatedRights = string.Empty;
while(expression is BinaryExpressionSyntax binaryExpressionSyntax)
{
if (GetLiteralExpressionValue(binaryExpressionSyntax.Right) is not { } right)
{
break;
}
concatenatedRights = right + concatenatedRights;
if (GetLiteralExpressionValue(binaryExpressionSyntax.Left) is { } left)
{
stringLiteralExpression = left + concatenatedRights;
return true;
}
expression = binaryExpressionSyntax.Left;
}
stringLiteralExpression = null;
return false;
}
static string? GetLiteralExpressionValue(
ExpressionSyntax expression)
{
if(expression is LiteralExpressionSyntax literal &&
literal.IsKind(SyntaxKind.StringLiteralExpression))
{
return literal.Token.ValueText;
}
return null;
}
I've tested this with 2, 5 and 6 concatenations.
I'm not planning on creating a PR since we're not going to use POExtractor after all. I'm writing our own tool, extracting to JSON (and using type detection instead detecting by variable name, and so far it works good, it's just a bit slower).
I'm writing our own tool, extracting to JSON (and using type detection instead detecting by variable name, and so far it works good, it's just a bit slower).
As I said before you can skip the limitation of the variable names, check #92
I'm writing our own tool, extracting to JSON (and using type detection instead detecting by variable name, and so far it works good, it's just a bit slower).
As I said before you can skip the limitation of the variable names, check #92
yes, thanks, but as I said before, I don't like it - because it actually doesn't eliminate the limitation, it just broadens the list of possible values. And again, if you ever write an analyzer for this, you can only analyze it if you check the types, not just the name, and you just end up with an unnecessarily complicated solution: checking names and types - where checking the types would suffice.
Also, my solution respects the <T> parameter, another considerable advantage - in my opinion. And again something you can only achieve by dealing with types.
Of course there's two downsides to my solution as well:
- only works when the solution can be compiled
- takes longer
In my environment I anticipate these downsides as having minimal impact.
Feel free to close the issues I've reported.