vendure-ecommerce / ngx-translate-extract

Extract translatable (using ngx-translate) strings and save as a JSON or Gettext pot file
MIT License
51 stars 19 forks source link

Feature: Keep empty translations when using key as default value #48

Closed stevengunneweg closed 6 months ago

stevengunneweg commented 6 months ago

I was happy to find that you have taken over maintenance on this library, I had a fork myself but it was quite some work to keep it up-to-date with changes made to Angular.

In my fork I added a feature that keeps empty translations empty when using the --key-as-default-value flag. It would be great if you could add this, possibly with an additional flag if desired.

I made the change to src/post-processors/key-as-default-value.post-processor.ts. Here I added a check to the mapping to see if the key was already present and thus intentionally left/made empty.

return draft.map((key, val) => (val === '' && existing.get(key) === undefined ? key : val));

My fork is a bit outdated so in this repo I believe it would become something like this

public process(draft: TranslationCollection, extracted: TranslationCollection, existing: TranslationCollection): TranslationCollection {
    return draft.map((key: string, val: TranslationInterface): TranslationInterface => val.value === '' && !existing.has(key) ? {value: key, sourceFiles: (val?.sourceFiles || [])} : val);
}

I opened a PR to add this feature

michaelbromley commented 6 months ago

Hi!

Can you explain in a bit more detail the use-case of this flag? I'm not sure I fully understand the effect it has.

stevengunneweg commented 6 months ago

Thanks for your quick response, of course.

In my project I have a few translations that for a certain language need to not show. E.g. for EN language a description should show but for NL language it's not necessary. The only way to check if we should show this specific translation is if the value is an empty string (falsy). We also have translations that still need to be translated, the ones where the value is equal to the key path.

Since I'm using the --key-as-default-value flag I would expect translations to only be filled with the key path when they do not yet exist and keep the existing value if they are updated manually.

In the example below I would expect key3 to remain an empty string. However with the current implementation key3 is filled with the key path every time I would run the extract command.

{
    "group": {
        "key1": "Translated value 1",
        "key2": "group.key2", // untranslated
        "key3": "" // intentionally empty
    }
}

The change I suggested in my PR makes sure that key3 remains an empty string but new translations are still added with the key path as their value.

sod commented 6 months ago

Thats indeed strange that the --key-as-default-value one overwrites "". The other two (null and string) don't do that.

I agree that this seems to be a bug. But .... if you want to do the maintainer a favor to not have to deal with a followup back and forth because it broke someones elses workflow, you'd create a new default value processor. E.g. I'd create a new KeyAsInitialDefaultValue used via --key-as-initial-default-value and leave this class untouched.

stevengunneweg commented 6 months ago

I think it's a good idea to build it in a non-breaking way 👍

I'll try to update my PR to apply it as per your suggestion as soon as I have some time

stevengunneweg commented 6 months ago

I've updated the PR. A new postprocessor has been added along with tests and updates to cli.ts and README.md.

I hope I set the cli commands correctly but I have no experience with the conflicts properties of yargs so these changes should be reviewed more thoroughly.

If there's any other requests or required changes, please let me know

michaelbromley commented 6 months ago

Thanks for the explanation & update to the PR! This will be released shortly.

stevengunneweg commented 5 months ago

I just updated to the latest version instead of my fork and everything seems to work as expected. Thanks for your help and ideas!