BartyCrouch icon indicating copy to clipboard operation
BartyCrouch copied to clipboard

Fix multiple code paths

Open mman opened this issue 3 years ago • 2 comments

Fixes https://github.com/FlineDev/BartyCrouch/issues/264

Proposed Changes

I have adjusted the logic of incrementalCodeUpdate method to properly handle extracting strings from source files specified via multiple entries in update.code.codePaths option.

In the past, the logic was to walk all codePaths directories. For each directory create a temporary .strings file. Merge all entries from the temporary .strings file into output. Invoking this logic for multiple codePaths in additive=false mode would repeatedly overwrite the output Localized.strings file with only the entries extracted from currently processed directory and would remove everything else.

The new logic uses only one temporary .strings file into which all entries from all codePaths are extracted by using extractLocStrings -a (the append mode) and only after that are all strings from the temporary .strings file merged into the output Localizable.strings.

Changes

  • When invoking extractLocStrings specify the -a option to append extracted strings into temporary .strings file instead of replacing them.
  • Use just one temporary .strings file for all strings extracted during code.update

Testing

I have a testing project with the following .bartycrouch.toml Multiple entries via update.code.codePaths, and update.code.additive=false Each subdirectory contains one NSLocalizedString entry.

[update]
tasks = ["code", "normalize"]

[update.code]
codePaths = ["Sources/TestBartyCrouch", "Sources/TestBartyCrouch1"]
localizablePaths = ["."]
additive = false

[update.normalize]
paths = ["."]
sourceLocale = "en"

[lint]
paths = ["."]
subpathsToIgnore = [".git", "carthage", "pods", "build", ".build", "docs"]
duplicateKeys = true
emptyValues = true

Invoking published release against this project results in only one entry detected:

€ rm en.lproj/Localizable.strings && touch en.lproj/Localizable.strings

€ bartycrouch update -v                                                
Starting Task 'Update Code' ...
2022-10-11 16:37:39.523: ℹ️  Incrementally updated keys of file '/Users/mman/Developer/TestBartyCrouch/en.lproj/Localizable.strings'.
2022-10-11 16:37:39.527: ✅  Sources/TestBartyCrouch:  Successfully updated strings file(s) of Code files.
2022-10-11 16:37:39.596: ℹ️  Incrementally updated keys of file '/Users/mman/Developer/TestBartyCrouch/en.lproj/Localizable.strings'.
2022-10-11 16:37:39.597: ✅  Sources/TestBartyCrouch1:  Successfully updated strings file(s) of Code files.
Task 'Update Code' took 0.144 seconds.
Starting Task 'Normalize' ...

€ cat en.lproj/Localizable.strings 
/* Greeting from TestBartyCrouch1 target */
"Hello, World, 1!" = "";

Invoking version with the new logic results in:

€ rm en.lproj/Localizable.strings && touch en.lproj/Localizable.strings
€ ../BartyCrouch/.build/debug/bartycrouch update -v
Starting Task 'Update Code' ...
2022-10-11 16:38:53.051: ✅  Sources/TestBartyCrouch:  Successfully updated strings file(s) of Code files.
2022-10-11 16:38:53.120: ✅  Sources/TestBartyCrouch1:  Successfully updated strings file(s) of Code files.
2022-10-11 16:38:53.123: ℹ️  Incrementally updated keys of file '/Users/mman/Developer/TestBartyCrouch/en.lproj/Localizable.strings'.
Task 'Update Code' took 0.141 seconds.
Starting Task 'Normalize' ...

€ cat en.lproj/Localizable.strings 
/* Greeting from TestBartyCrouch target */
"Hello, World!" = "";

/* Greeting from TestBartyCrouch1 target */
"Hello, World, 1!" = "";

mman avatar Oct 11 '22 14:10 mman

@mman Thank you for this. Just want to let you know that this is on my radar and I quickly skimmed through the changes and noticed that this needs a bit more detailed check, there are some things you've changed here which require a more detailed look to not break things. I don't have enough time right now to do this, but I'll get back to it as soon as possible.

Feel free to ping me if you don't hear from me after 10 days.

Jeehut avatar Oct 13 '22 21:10 Jeehut

@Jeehut We have just run into the same issue documented here https://github.com/FlineDev/BartyCrouch/issues/264 - if this PR fixes this, any chance we could have it merged please? ☺️

hactar avatar Apr 05 '24 09:04 hactar