[Bug] update.code with additive=false and code in multiple parent directories looses strings

[mman]

Steps to Reproduce:

I have my .bartycrouch.toml configured in a way that it looks up strings in multiple code directories referenced via ../ and combining them into one Localizable.strings file located in ./

For example like this:

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

[update.interfaces]
paths = ["../App"]
defaultToBase = false
ignoreEmptyStrings = false
unstripped = false

[update.code]
codePaths = ["../App", "../Shared", "../Shared2"]
subpathsToIgnore = ["Vendor"]
localizablePaths = ["."]
defaultToKeys = false
additive = false
unstripped = false
plistArguments = true
customFunction = "LocalizedStringResource"

[update.normalize]
paths = ["."]
sourceLocale = "en"
harmonizeWithSource = true
sortByKeys = true

[lint]
paths = ["."]
duplicateKeys = true
emptyValues = true

Expected Behavior:

I expect bartycrouch update to combine all strings from all sources, clean them up and put them into one Localizable.strings file.

Current Behavior:

in additive=false mode strings from all directories but last specified via codePaths are lost and only strings for the last code directory are output.

This means that all strings from ../App and ../Shared directories are lost and only strings found in ../Shared2 are actually output. Not sure if it is related to the fact that I use parent dir ../ references.

Workaround:

For now I have found a workaround to specify just one entry in codePaths and skip unwanted stuff via subpathsToIgnore.

Environment

Show environment details

€ bartycrouch --version
Version: 4.11.0

[Jeehut]

@mman Thank you for reporting this, being detailed and even providing a workaround. I think it makes sense to change the default behavior to work like what you expect to work like. I don't think it's related to the ../ prefixes (if they work at all, they should work correctly, and they seem to work for your). Instead, I just checked and found the function which holds the logic for updating the Strings files and found that this function has a for-loop right at the start spanning the entire function. So on each iteration, the resulting Strings file is created newly. Instead, all paths should be first walked through. All Strings files should be combined into one. Then, the rest of the logic should be run.

I don't currently have the time to implement this and it seems not to be urgent as you found a workaround. But if you're up to implementing this with a test that fails with the currnent behavior but passes with the changes, I'd happily review and merge.

[mman]

I have taken a quick look at the linked code, will see if I can come up with a failing test first and then fix the logic... it's like a low priority with existing workaround, but still would be nice get it working... bartycrouch is helping me a lot and I do not see it disappear any time soon given where all the l18n stuff is going on Apple platforms...

[mman]

@Jeehut How do you swift build and swift test usually... On my swift 5.7 system the build/test fails:

€ swift test --disable-sandbox
Building for debugging...
/Users/mman/Developer/BartyCrouch/Tests/BartyCrouchTranslatorTests/Secrets/Secrets.swift:8:33: error: 'module' is inaccessible due to 'internal' protection level
    let secretsFileUrl = Bundle.module.url(forResource: "secrets", withExtension: "json")
                                ^~~~~~
/Users/mman/Developer/BartyCrouch/.build/x86_64-apple-macosx/debug/MungoHealer.build/DerivedSources/resource_bundle_accessor.swift:4:16: note: 'module' declared here
    static let module: Bundle = {
               ^
[2/5] Compiling BartyCrouchTranslatorTests Secrets.swift
error: fatalError

[Jeehut]

@mman The same command works for me, but the issue is that there's an asset file missing with secrets for testing machine translators. It's my fault, I hadn't documented it in the README, but I've just added the following to the "Contributing" section which should solve your problem:

In order for the tests to run build issues, you need to run – also add an an API key in the new file to run the translations tests, too:

cp Tests/BartyCrouchTranslatorTests/Secrets/secrets.json.sample Tests/BartyCrouchTranslatorTests/Secrets/secrets.json

Thanks for taking a look at this!

[mman]

@Jeehut I have attempted to fix this via #267. I tested against my limited setup, works as expected but I am not sure if I did not accidentally break anything else. swift test passes.