Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit folders on the codePaths argument shouldn't be ignored later. #239

Closed
tladesignz opened this issue Dec 21, 2021 · 5 comments
Closed

Comments

@tladesignz
Copy link

Problem Statement

I have code in a pod which I want searched, but BartyCrouch will ignore these files, because they'll contain a "pods" folder in their URL.

Suggested Solution

From user view:

When a folder is added in the codePaths argument, which contains ignored folders, then the files in there should be included anyways.

From developer view:

Only the subfolders of the folders stated in the codePaths argument should be checked against
https://github.com/Flinesoft/BartyCrouch/blob/main/Sources/BartyCrouchKit/FileHandling/CodeFilesSearch.swift#L11

(BTW: This list should be static!)

That method probably needs a ignoredBase parameter, which then is used in truncating the url parameter before the check.

Example Usage

[update.code]
codePaths = ["Pods/MySubprojectPod"]

=> This should search all code files below that folder, despite CodeFilesSearch.shouldSkipFile(at:) returning true (today).

Possible Involvement

  • I could help with implementation: ✅
  • I could help with testing: ✅
@Jeehut
Copy link
Member

Jeehut commented Jan 3, 2022

@tladesignz Thank you very much for reporting this. We have a similar issue with InfoPlist.strings files getting ignored due to fixed constants that can't be configured to not be ignored via the configuration file. I plan to introduce configuration options for all fixed strings, I can also do that for Pods files so you can override ignoring the entire Pods folder and instead specify your own to ignore. The new config options will have to default to the current values though so there's no break in the behavior.

If what you want is something different and you actually want to change the behavior of codePaths, feel free to post a PR with your exact suggestion though and I will have a look, I'm open to merge that, too. I'm just not sure if I will have the time to implement it (I'm not sure if it's an easy fix or needs some logic refactoring).

@tladesignz
Copy link
Author

Making the ignore list configurable certainly would give a tool to fix my problem.

However, I would still consider the usage counter-intuitive: Even though, I explicitly add a certain folder to get scanned, it gets ignored by default, because one of its parent folders matches the default ignore-list.

As already stated: The ignore-list should only be applied to automatically found sub-folders of given codePaths folders.

I understand that changing that behaviour might break a very small fraction of configurations. However I'd argue, that these were done in error and never fixed because the current implementation hides the problem. There's no situation I can imagine where it does make any sense to add a folder to the codePaths which gets ignored completely, anyway.

This change will be easily spotted and can be easily fixed in such a faulty configuration. After a BartyCrouch updated, your localization source file will contain a whole lot of new strings you didn't expect. Fix: remove the unwanted codePaths folder. Re-build. Done.

So, actually one can argue, that the proposed change in the behaviour of BartyCrouch doesn't break configurations, but finally helps users to spot faulty configurations. That's actually wanted.

I'll contribute a change after my vacation. Happy new year!

@Jeehut
Copy link
Member

Jeehut commented Jan 4, 2022

@tladesignz I think you got me wrong, I never said that I think your suggestion doesn't make sense or would break things for users, sorry that I wasn't more explicit: I do agree with your assessment that an explicit mention of a (sub)path in the codePaths should not be ignored by global ignores. The global ignores should only happen for subpaths if anything. I'm just not sure if that's easy to implement or not and since I'm planning a larger rewrite anyways, it's not worth fixing for me before that. But feel free to give it a try. 👍

tladesignz added a commit to tladesignz/BartyCrouch that referenced this issue Jan 11, 2022
…even though they might contain otherwise ignored folders as their parent folders.
@tladesignz
Copy link
Author

Was actually quite easy to fix. Hope that helps!

FlineDevPublic pushed a commit that referenced this issue Jan 14, 2022
Fixed issue #239: Don't ignore explicitly mentioned folders, even though they might contain otherwise ignored folders as their parent folders.
@FlineDevPublic
Copy link
Collaborator

Fixed via #233 and #240. 🎉

Thank you @tladesignz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants