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

Allow escaping linked translations #248

Merged
merged 5 commits into from
Oct 1, 2024

Conversation

Fasust
Copy link
Contributor

@Fasust Fasust commented Sep 30, 2024

I ran into an issue with linking translations and having them immediately followed by another word character.

Example:

{
  "common": {
    "nbsp": "\u00a0",
    "@nbsp": "No-Break Space https://unicode-explorer.com/c/00A0",
  },
   "message": "10@:common.nbspDays"
}

Message is linking to @:common.nbsp, but the link will be interpreted as @:common.nbspDays which is invalid.
My PR adds the option to escape linked translations:

{
  "common": {
    "nbsp": "\u00a0",
    "@nbsp": "No-Break Space https://unicode-explorer.com/c/00A0",
  },
   "message": "10@:{common.nbsp}Days"
}

@Tienisto
Copy link
Member

Hi, thanks for the PR!
Do you know how other solutions handle this issue? This feature is inspired by linked translations of easy_localization but they probably followed the structure of Vue i18n.

I think it is better to handle it similar to how Dart handles interpolation ($ for greedy interpolation, ${} for edge cases).
Therefore, the following syntax might be safer to use (e.g. when somebody wants to have an actual @ in the string right after the linked translation):

{
  "message": "10@:{common.nbsp}Days"
}

Here, I surrounded the path with {...}
Let me know how you think about this. It might be a little bit more difficult to implement though.

@Fasust
Copy link
Contributor Author

Fasust commented Oct 1, 2024

Thank you for the feedback @Tienisto
I updated the code to use your suggested escaping syntax

slang/lib/src/builder/utils/regex_utils.dart Outdated Show resolved Hide resolved
slang/README.md Outdated Show resolved Hide resolved
@Tienisto
Copy link
Member

Tienisto commented Oct 1, 2024

Please add the following tests in StringTextNode/StringInterpolation.braces (that will fail):

test('with links', () {
      final test = r'{myArg} @:myLink';
      final node = textNode(test, StringInterpolation.braces);
      expect(node.content, r'${myArg} ${_root.myLink}');
      expect(node.params, <String>{'myArg'});
    });

test('with escaped links', () {
      final test = r'{myArg} @:linkA @:{linkB}hello @:{linkC}';
      final node = textNode(test, StringInterpolation.braces);
      expect(node.content,
          r'${myArg} ${_root.linkA} ${_root.linkB}hello ${_root.linkC}');
      expect(node.params, <String>{'myArg'});
});

The main problem here is that we parse the linked translations after the interpolation ({myArg}). Therefore, @:{myLink} is parsed as an interpolation argument instead of a link

@Tienisto
Copy link
Member

Tienisto commented Oct 1, 2024

You probably need to parse the linked translations first and update replaceBracesInterpolation to add exclusion for matches starting with @:. Let me know if you need help

@Fasust
Copy link
Contributor Author

Fasust commented Oct 1, 2024

Good catch @Tienisto , I added the tests you recommended. As far as I understand, ignoring matches leading with @: in replaceBracesInterpolation has solved the issue.

@Tienisto
Copy link
Member

Tienisto commented Oct 1, 2024

Nice!

@Tienisto Tienisto merged commit 3fb77d9 into slang-i18n:main Oct 1, 2024
3 checks passed
Tienisto pushed a commit that referenced this pull request Oct 1, 2024
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

Successfully merging this pull request may close these issues.

3 participants