Skip to content

Rely on gitignore to exclude listed files in ghcide (#4665)#4736

Merged
fendor merged 4 commits into
haskell:masterfrom
blackheaven:ghcide/use-gitignore
Nov 11, 2025
Merged

Rely on gitignore to exclude listed files in ghcide (#4665)#4736
fendor merged 4 commits into
haskell:masterfrom
blackheaven:ghcide/use-gitignore

Conversation

@blackheaven

Copy link
Copy Markdown
Contributor

This aimes to fix #4665.

Note that I directly calling git, as th gitignore format is non-trivial.

I can roll a partial parsing implementation if requested.

@blackheaven
blackheaven requested a review from wz1000 as a code owner October 4, 2025 16:42
@blackheaven
blackheaven force-pushed the ghcide/use-gitignore branch 2 times, most recently from 013cd7b to 5db38c5 Compare October 4, 2025 16:52
@blackheaven

Copy link
Copy Markdown
Contributor Author

I don't think the CI failure is related to my PR.

@sgillespie

Copy link
Copy Markdown
Collaborator

I don't think the CI failure is related to my PR.

Looks like a flaky thing, it fixed itself

@blackheaven

Copy link
Copy Markdown
Contributor Author

Thanks, let's try again

@fendor fendor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Small comments, otherwise LGTM!

Unfortunately, there are not automated tests for this cli mode, we'd need to add some, but that's some fair amount of work...

Comment thread ghcide/src/Development/IDE/Main.hs Outdated
Comment thread ghcide/src/Development/IDE/Main.hs Outdated

@fendor fendor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Couple of issues when trying this patch on the HLS codebase, but otherwise I like the direction!

Comment thread ghcide/src/Development/IDE/Main.hs Outdated
Comment thread ghcide/src/Development/IDE/Main.hs Outdated
@blackheaven
blackheaven force-pushed the ghcide/use-gitignore branch 2 times, most recently from dfa2415 to cc29abb Compare October 29, 2025 11:54
@blackheaven

Copy link
Copy Markdown
Contributor Author

I won't lie, I have no clue about the broken pipe on windows error is related to my change.

@fendor

fendor commented Oct 29, 2025

Copy link
Copy Markdown
Collaborator

@blackheaven it is not caused by you. As already said, iirc there is not a single test case for the code path you are changing, so you can safely ignore any CI failures except pre-commit and -Werror :D

@blackheaven

Copy link
Copy Markdown
Contributor Author

Thanks a lot

@fendor fendor added the status: needs review This PR is ready for review label Oct 29, 2025
Just _ -> \path -> do
let lookups =
if takeExtension path `elem` [".hs", ".lhs"]
then [path]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question, what should happen if a user explicitly asks for a file to load, but the file is .gitignored?

Should we perhaps still load it? I feel like, that might be more natural behaviour, but I am admittedly not sure.
If we do want to ignore ignored files, then I would still suggest that we should warn the user in this case, that we ingored a file they requested.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We could add an option in the CLI to by-pass gitignore, but I guess it'll be more complex.

Any idea which form the warning would take?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The warning should probably just be printed via the Recorder, so nothing fancy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here we are!

@fendor fendor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thank you!

@fendor
fendor enabled auto-merge (squash) November 10, 2025 10:42
@fendor

fendor commented Nov 11, 2025

Copy link
Copy Markdown
Collaborator

Our CI is broken for good right now, @wz1000 is looking into it, but we don't know how long it is going to be broken...

@fendor
fendor disabled auto-merge November 11, 2025 13:02
@fendor
fendor merged commit 1078d4b into haskell:master Nov 11, 2025
58 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs review This PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

typecheck should ignore .gitignore-ed files

3 participants