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.

Sign up for GitHub

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

Jump to bottom

The jsToXliff12 lib creates xliff with wrong format in case of falsy source #57

Closed
AdriAt360 opened this issue Feb 26, 2024 · 3 comments · Fixed by #58
Closed

The jsToXliff12 lib creates xliff with wrong format in case of falsy source #57

AdriAt360 opened this issue Feb 26, 2024 · 3 comments · Fixed by #58

Comments

@AdriAt360
Copy link
Contributor

🐛 Bug Report

The jsToXliff12 lib creates xliff with wrong format in case of falsy source

To Reproduce

Below is an example, what is important is to provide a resource with a falsy source, e.g. an empty string

const resources = {
    namespace: {
        field: { source: "", target: "translation" }
    }
};
const xliffDescriptor = {
    resources,
    sourceLanguage: "en",
    targetLanguage: "fr"
};
const xliffFile = await xliff.jsToXliff12(xliffDescriptor);

xliffFile contains something like below, with no source tag, which is not valid against the specification, image

  <file original="namespace" datatype="plaintext" source-language="en" target-language="fr">
    <body>
      <trans-unit id="field">
        <target>translation</target>
      </trans-unit>
    </body>
  </file>

Expected behavior

The expected result should include something like below

  <file original="namespace" datatype="plaintext" source-language="en" target-language="fr">
    <body>
      <trans-unit id="field">
        <source></source>
        <target>translation</target>
      </trans-unit>
    </body>
  </file>

Your Environment

  • runtime version: node v20
  • os: Mac, Windows, Linux
  • any other relevant information
@adrai
Copy link
Contributor

adrai commented Feb 26, 2024

Feel free to provide a PR.

@AdriAt360
Copy link
Contributor Author

@adrai I have looked into this, because the change sounds simple enough (change this to if (unit.source !== undefined) (same for the version 1.2)
But when I do this, it breaks many tests, and I'm not knowledgeable enough of xliff to know if it's expected or not
if it's ok for you, I'll provide a PR soon and we can discuss there if needed

@AdriAt360 AdriAt360 mentioned this issue Feb 26, 2024
Merged
4 tasks
@AdriAt360
Copy link
Contributor Author

@adrai I just opened a PR for my suggested fix, if you want to give it a look and provide any feedback 🙏🏽

@adrai adrai closed this as completed in #58 Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure jsToXliff functions generate valid xliff files adridevlyon/xliff
2 participants
@adrai @AdriAt360