js-lingui icon indicating copy to clipboard operation
js-lingui copied to clipboard

fix(extract): when files are used, don't overwrite obsolete

Open nicolas-cherel opened this issue 1 year ago • 13 comments

Description

When using lingui extract <...files> all translations in catalog that are not in the listed file get their obsolete flag to false, regardless if the obsolete flag was not set or already set to true.

Types of changes

  • [x] Bugfix (non-breaking change which fixes an issue)

Fixes # (issue) #1963

Checklist

  • [x] I have read the CONTRIBUTING and CODE_OF_CONDUCT docs
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] I have added the necessary documentation (if appropriate)

nicolas-cherel avatar Jun 21 '24 11:06 nicolas-cherel

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
js-lingui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2024 8:31am

vercel[bot] avatar Jun 21 '24 11:06 vercel[bot]

size-limit report 📦

Path Size
./packages/core/dist/index.mjs 2.88 KB (0%)
./packages/detect-locale/dist/index.mjs 723 B (0%)
./packages/react/dist/index.mjs 1.68 KB (0%)
./packages/remote-loader/dist/index.mjs 7.26 KB (0%)

github-actions[bot] avatar Jun 21 '24 11:06 github-actions[bot]

@nicolas-cherel thanks for the contribution! Please include tests that cover this use case

andrii-bodnar avatar Jun 21 '24 12:06 andrii-bodnar

i would recommend to add full e2e test here packages/cli/test

  1. Create a folder with test case name
  2. In fixtures put test files to extract (you can copy from sibling test case)
  3. in existing put catalogs with already extracted messages an obsolete flag
  4. in expected put how catalogs should like after extracting
  5. add separate describe into packages/cli/test/index.test.ts and run extractCommand with parameters you need

timofei-iatsenko avatar Jun 21 '24 12:06 timofei-iatsenko

Note: it seem that only the lingui json format has support for the obsolete flag, I hope depending on it for the tests is not a issue

nicolas-cherel avatar Jun 24 '24 10:06 nicolas-cherel

Po files also support obsolete, i would prefer to have it with PO files, lingui json format is not recommended to use by docs.

timofei-iatsenko avatar Jun 24 '24 11:06 timofei-iatsenko

Ok I must have missed something on my first pass, I'll get right away now that I have the json version set up

nicolas-cherel avatar Jun 24 '24 13:06 nicolas-cherel

Ok, note, we previously had an issue specific to the json files where a "obsolete": true was added to translations with partial extract, which does not happen with .po files due to the format specificities. I manage to witness the obsolete flag being removed with the partial extract though, so that fix is confirmed.

I renamed the test suite fixtures/exisiting/expected folder to extract-partial-consitency as the general problem was more a consistency issue with whole and partial extracts generating inconsistent outputs.

nicolas-cherel avatar Jun 24 '24 14:06 nicolas-cherel

re-pushed for a typo fix on extract-partial-consistency folder

nicolas-cherel avatar Jun 25 '24 10:06 nicolas-cherel

@nicolas-cherel could you please address the discussion above?

andrii-bodnar avatar Aug 01 '24 06:08 andrii-bodnar

sorry, I was not available on july, and forgot to get back to the issue, I'll try to do it ASAP

nicolas-cherel avatar Sep 06 '24 12:09 nicolas-cherel

Hi @nicolas-cherel, is there anything we can do to move this PR forward?

andrii-bodnar avatar Oct 14 '24 12:10 andrii-bodnar

So so sorry this got off my radar, I took the quick route with a in github edit, not my proudest moment but it was the best way for me to advance that right now.

nicolas-cherel avatar Oct 16 '24 07:10 nicolas-cherel

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.60%. Comparing base (d6b9698) to head (2ecc64d). Report is 71 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1964      +/-   ##
==========================================
- Coverage   76.65%   75.60%   -1.05%     
==========================================
  Files          81       88       +7     
  Lines        2090     2173      +83     
  Branches      533      552      +19     
==========================================
+ Hits         1602     1643      +41     
- Misses        375      419      +44     
+ Partials      113      111       -2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 21 '24 08:10 codecov[bot]

@nicolas-cherel thank you!

andrii-bodnar avatar Oct 23 '24 11:10 andrii-bodnar