daniel-sc / ng-extract-i18n-merge

Extract and merge i18n xliff translation files for angular projects.
MIT License
170 stars 18 forks source link

includeContext option description incorrectly refers to notes tag #95

Closed co-dax closed 7 months ago

co-dax commented 7 months ago

At the following link https://github.com/daniel-sc/ng-extract-i18n-merge?tab=readme-ov-file#configuration the docs are saying the following in the context of includeContext option:

Whether to include the context information (like notes) in the translation files. This is useful for sending the target translation files to translation agencies/services. When sourceFileOnly, the context is retained only in the sourceFile.

...but the note tag in preserved in the destination/merged file which should be the correct behaviour since note tag in outside of context-group to which the property includeContext is (at least should be) referring.

So I would say that the excerpt:

(like notes)

...should be removed from the link I reported above.

doggy8088 commented 7 months ago

I found this issue today. I think the note should not be removed and the note tag is not contains in <context-group> tag.

The <note> tag is very important for translation. It should be keeped.

daniel-sc commented 7 months ago

@co-dax , @doggy8088 I'm not sure I understand your point(s).

  1. Which version(s) are you using?
  2. I'm pretty sure the documentation matches the actual behavior: when includeContext: false (the default value), then the note node is not included in the translation files. Can you please double check and/or give a reproduction?
  3. Maybe this is more about naming and/or defaults? The configuration includeContext does not specifically adress the xml node context-group, but "contextual information" such as location/meaning/description (e.g. with XLF2 naming of nodes is different).

Or this this about something else entirely?

doggy8088 commented 7 months ago

@daniel-sc Please check my repo: https://github.com/doggy8088/ng-i18n-demo/tree/ng-extract-i18n-merge

At first, check this commit: https://github.com/doggy8088/ng-i18n-demo/commit/a45ff6d2d9fd9174f5a880accbe3274f5155baa3?diff=unified&w=0

After I run ng add ng-extract-i18n-merge and ng extract-i18n command. The <note> has been removed.

image

https://github.com/doggy8088/ng-i18n-demo/commit/a45ff6d2d9fd9174f5a880accbe3274f5155baa3?diff=unified&w=0#diff-e2cae9b9c1350d0b44c90db489eb38f00c905a1ecd9e3a7c52988c389d0730faR9-R12

I think the <context-group> should be removed by default, but the <note priority="1" from="description">放置在首頁的招呼語</note> should be kept. It's different.

co-dax commented 7 months ago

@daniel-sc I was using version 1.4 and with that version it used to work as I described above. I have just upgraded to version 2.9.1 and now it works as described by @doggy8088 just above. I thnk @doggy8088 is right in his last comment on how it should be working.

daniel-sc commented 7 months ago

@doggy8088 @co-dax ok, finally I got your point :) Would one of you be motivated to make a PR for this bug?

doggy8088 commented 6 months ago

@daniel-sc Good job! 😃 Thanks! 👍