mashpie / i18n-node

Lightweight simple translation module for node.js / express.js with dynamic json storage. Uses common __('...') syntax in app and templates.
MIT License
3.09k stars 421 forks source link

Supporting standalone pipe characters #435

Closed matthub closed 4 years ago

matthub commented 4 years ago

We are happy users of your great library, but we encountered an issue with the pipe character. We wanted to use the pipe symbol directly as part of the text - without using the variable workaround as mentioned in this stackoverflow question. Thus this pull request is changing the parsing logic regarding to the pipe as following:

  1. only an interval with starting and closing brackets is evaluated (limiting within the regex)
  2. to treat a single pipe symbol as "otherwise" case, there must be already an interval specified before

Additionally I've pushed the version to 0.10.1. Existing tests ran without any changes, additionally I added two test cases to verify the changed standalone pipe symbol behavior.

This should fix #274 and could maybe superseed pull request #388

If there is anything to add in the code or additionally to test I'm happy to do that. Thanks for the great library!

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.01%) to 97.787% when pulling 06be2281b7868c088a1b5a354a74dfed3422d699 on matthub:master into fb9aa221d74a22ba81ebdc3f7bbc859fac93ccb4 on mashpie:master.

matthub commented 3 years ago

@AndreiSoroka Yes, and there were some workarounds presented which only partially worked for our situation. Can you elaborate a little bit more - like what question you have, or if that code does not work for you?

AndreiSoroka commented 2 years ago

@matthub Oh. Sorry. That MR was 2020 :)

I'm from https://github.com/mashpie/i18n-node/issues/274

You wrote

This should fix #274"

But that problem is actual.

matthub commented 2 years ago

@AndreiSoroka sorry for the long delay, just tried to put your examples in a test case, which ran through. Can you provide a simple test case to reproduce?

  it('AndreiSoroka 1 - should ignore standalone pipe', function() {
    const standalone = 'page number  3 | My site';
    should.equal(
       pluralTest.__(standalone),
       standalone
    );
  });

  it('AndreiSoroka 2 - should support escapes', function() {
    const standalone = 'page number  3 \\| My site';
    should.equal(
       pluralTest.__(standalone),
       standalone
    );
  });

  it('274 example - should ignore single standalone | symbol', function() {
    const standalone = 'Foo Bar | Baz Beep';
    should.equal(
       pluralTest.__(standalone),
       standalone
    );
  });
AndreiSoroka commented 2 years ago

Hey Thanks for the reply, I'll try to reproduce it next week

flatoy commented 2 years ago

I was able to use "someKey": "hello %7C there" and decodeURI(t("somekey")) which becomes hello | there

edit: commented on wrong issue