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

Remove registry-foreign dependency and use the new Spago.Glob module instead. #1220

Merged
merged 4 commits into from
Apr 26, 2024

Conversation

Blugatroff
Copy link
Contributor

Description of the change

Follow up of #1210

I replaced the only remaining use of Registry.Foreign.FastGlob.match with Spago.Glob.gitignoringGlob.

After running tests i noticed, that compileGlob is called with paths into the .spago directory, which is ignored by a .gitignore file, which means gitignoringGlob never found a match.
Before using a .gitignore file for pruning the search, gitignoringGlob now checks that none of the includePatterns would be excluded.

gitignorePatternToMicromatch also had a bug, fooin a gitignore should translate to a micromatch **/foo/** to properly match stuff like foo/bla/blob.

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊

Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

src/Spago/Glob.purs Outdated Show resolved Hide resolved
@f-f
Copy link
Member

f-f commented Apr 26, 2024

@Blugatroff ah, Windows CI is not happy. I guess this is about paths containing \ instead of the / we match on 😄

@f-f f-f merged commit 82d99ab into purescript:master Apr 26, 2024
3 checks passed
@f-f f-f mentioned this pull request Jul 6, 2024
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