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

Do not treat trailing # in gitignore as comment #797

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Oct 2, 2024

Just started using knip today and it was instrumental in some really nice cleanup in our codebase. I hit #795 right off the bat, though, and it took me a while to figure out what was going on. I figured I could say "thanks" by opening up a quick PR to fix the issue!

The logic in the gitignore parser that strips trailing # ... off of .gitignore lines appears to be wrong. In my testing, you cannot add a comment to the end of a .gitignore line in this manner (syntax highlighting be damned):

# this is a comment
dist # this is not a comment

This tracks with git's documentation (emphasis mine) as well as my own messing around with git:

A line starting with # serves as a comment. Put a backslash (\) in front of the first hash for patterns that begin with a hash.

The previous parsing logic (added in e455527) stripped all non-backslashed #... out. I updated the logic to only strip a leading \, if it's in front of a hash, so the following cases will be handled properly:

trailing-hash-ignore#
\#leading-hash-ignore
just#some-file

Copy link

pkg-pr-new bot commented Oct 3, 2024

Open in Stackblitz

bun add https://pkg.pr.new/knip@797

commit: 9b72edb

@webpro
Copy link
Collaborator

webpro commented Oct 3, 2024

Thanks @mcous! Very much appreciate you made the leap from "dang this thing doesn't work at all" to a PR with a fix with tests! Will be released shortly.

@webpro webpro merged commit 7fc6355 into webpro-nl:main Oct 3, 2024
25 checks passed
@webpro
Copy link
Collaborator

webpro commented Oct 3, 2024

🚀 This pull request is included in v5.31.0. See Release 5.31.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

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