Skip to content

Conversation

@rcasula
Copy link

@rcasula rcasula commented Mar 2, 2025

This MR will allow us to override the default Bundle when generating resources strings.
Fixes #111 and #121

@liamnichols
Copy link
Owner

Hi @rcasula, thank you so much for taking the time to open this PR! I appreciate that you are building on top of the config file change 🙇

I've been sitting on the fence about such customisation point since I want XCStrings Tool to generate the correct code in the first place rather than relying on the end-user having to figure things out. But at the same time, I do understand that this isn't always possible.

Is your motivation for this change to work around the issues with Previews (similar to the initial report in #111)? If so, are you able to help me reproduce it? I tried the original steps last year but I still didn't observe the issue.

If I could reproduce the issue myself, I'd want to introduce a similar Bundle.current myself inside the generated source file so that there is no need to define bundle_override in the first place.

That said, I know there are still other scenarios where people might want to use a different Bundle so I think we can still go ahead with this change. I'd just like to see if you can help me with the previews issue while I have you here 🙂

@rcasula
Copy link
Author

rcasula commented Mar 4, 2025

Hi @liamnichols, thank you for your answer. So interestingly I cannot reproduce it anymore 🤯
So I was using the repo more than a year ago, and it was crashing when using previews referencing resources from another target, but I don't remember which version of Xcode I was using. To be clear, it was not an issue with the repo itself, rather with spm/Xcode (?)
I tested it now with 16.2, 16.1, 15.4, 15.2 and I can't reproduce it anymore.
It's really weird because it was reproducible 100% of the times. Anyway, it's good that it's not happening anymore 😅

@liamnichols
Copy link
Owner

Yes it's very weird. I have a feeling that it might have been fixed as part of a macOS update. OR it only reproduced under certain conditions.

I'm going to keep this on-hold incase you do still reproduce the issue, but I won't merge it just yet 🙏

@liamnichols liamnichols deleted the branch liamnichols:ln/config-file April 2, 2025 07:21
@liamnichols liamnichols closed this Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants