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

includeContext option description incorrectly refers to notes tag #95

Closed
co-dax opened this issue Jan 18, 2024 · 6 comments · Fixed by #100
Closed

includeContext option description incorrectly refers to notes tag #95

co-dax opened this issue Jan 18, 2024 · 6 comments · Fixed by #100
Labels
bug Something isn't working

Comments

@co-dax
Copy link

co-dax commented Jan 18, 2024

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.

@co-dax co-dax added the bug Something isn't working label Jan 18, 2024
@doggy8088
Copy link

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
Copy link
Owner

@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
Copy link

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

At first, check this commit: doggy8088/ng-i18n-demo@a45ff6d?diff=unified&w=0

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

image

doggy8088/ng-i18n-demo@a45ff6d?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
Copy link
Author

co-dax commented Jan 29, 2024

@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
Copy link
Owner

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

@doggy8088
Copy link

@daniel-sc Good job! 😃 Thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants