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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect .git/info/exclude in addition to .gitignore #613

Closed
weilbith opened this issue Apr 30, 2024 · 10 comments
Closed

Respect .git/info/exclude in addition to .gitignore #613

weilbith opened this issue Apr 30, 2024 · 10 comments
Labels
feature request Feature request

Comments

@weilbith
Copy link

weilbith commented Apr 30, 2024

Hey 馃憢馃従

Thank you very much for this nice tool! It is great to see how it develops.

A problem I stumble over is the ignoring of developer local files. So I usually have a few files I don't want to be checked-in to the VCS, but as it is a "personal" thing, I add them to .git/info/exclude (see documentation) rather than polluting the shared .gitignore file. As knip already respects the .gitignore file, it would be amazing if it could to the same for the .git/info/exclude file too. As I don't wanna add these ignore patterns to .gitignore I don't wanna add them to knip it's ignore configuration either.
Right now I always have issues because knip complains in my git hooks and I can't rely on your amazing autofixing.

Happy to discuss this idea further. 馃檭
Thanks in advance!

@weilbith weilbith added the feature request Feature request label Apr 30, 2024
@webpro
Copy link
Collaborator

webpro commented Apr 30, 2024

Thanks @weilbith!

I didn't know about .git/info/exclude, that looks useful indeed.

For now, you could consider using a dynamic knip.ts, read .git/info/exclude, and its items to knip ignore patterns (or as negated entry/project patterns).

@weilbith
Copy link
Author

weilbith commented Apr 30, 2024

Wow, that was a fast reply! 馃槺
Thank you very much!

Ah, cool. That actually sounds like a sound idea!
I'll implement that for now. Do you think it will make it as a feature upstream? Should I contribute a PR?


In case someone stumbles across this issue and it is not (yet) implemented upstream, here is my version of adding it to my knip.ts configuration:

import { readFile } from "fs/promises";
import type { KnipConfig } from "knip";

// TODO: Remove once feature was implemented upstream.
async function readGitInfoExcludePatterns(): Promise<string[]> {
  const buffer = await readFile(".git/info/exclude");
  return buffer
    .toString("utf-8")
    .split(/\r\n|\r|\n/)
    .filter((line) => !!line.trim())
    .filter((line) => !line.startsWith("#"))
    .map((line) => (line.endsWith("/") ? `${line}**` : line));
}

export default async function composeConfiguration(): Promise<KnipConfig> {
  const gitInfoExcludePatterns = await readGitInfoExcludePatterns();

  return {
    ignore: [
      ...gitInfoExcludePatterns,
      // ... more patterns
    ],
    // ... more configuration
  };
}

@webpro
Copy link
Collaborator

webpro commented Apr 30, 2024

Thanks for sharing your solution!

Feel free to give this a shot. That part of the code is here: https://github.com/webpro/knip/blob/main/packages/knip/src/util/globby.ts#L59

@weilbith
Copy link
Author

Feel free to give this a shot. That part of the code is here: https://github.com/webpro/knip/blob/main/packages/knip/src/util/globby.ts#L59

Hui, that's a function 馃槃
Let me see. Thinking about how to integrate it best. Because the current logic seem to be quite evolved, acting on various levels including the search for .gitignore files, while there is always only a single .git/info/exclude. 馃

@webpro
Copy link
Collaborator

webpro commented Apr 30, 2024

Maybe you could consider something like moving everything inside the if clause of entryFilter to a separate function and call that function once for .git/info/exclude.

@webpro
Copy link
Collaborator

webpro commented May 3, 2024

@weilbith Is this still something you fancy picking up? I have absolutely no hurries with this one, just checking up.

@weilbith
Copy link
Author

weilbith commented May 8, 2024

@webpro I'm theoretically up for it. Just the typical excuse that I'm more than busy right now. Just became father and don't have the time anymore to do such things in my free time anymore. Sorry. 馃槙
And as the solution above works atm. the pressure is low. 馃檲

@webpro webpro closed this as completed in 29a0bdc May 10, 2024
@webpro
Copy link
Collaborator

webpro commented May 10, 2024

馃殌 This issue has been resolved in v5.14.0. See Release 5.14.0 for release notes.

Using Knip in a commercial project? Please consider sponsoring me.

@webpro
Copy link
Collaborator

webpro commented May 10, 2024

Thanks @weilbith! Wishing you good health for everyone 鉂わ笍

@weilbith
Copy link
Author

Thank you so much! My configuration just got more lean. 馃槉

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

No branches or pull requests

2 participants