Skip to content

fix: inverted gitignore behaviour for --dump-inputs #1882

Merged
thomas-zahner merged 2 commits intolycheeverse:masterfrom
rina-forks:fix-gitignore-dump-inputs
Nov 1, 2025
Merged

fix: inverted gitignore behaviour for --dump-inputs #1882
thomas-zahner merged 2 commits intolycheeverse:masterfrom
rina-forks:fix-gitignore-dump-inputs

Conversation

@katrinafyi
Copy link
Copy Markdown
Member

the use of no_ignore was missing a ! before being passed as skip_ignored. i figure there was some confusion about the meaning of "no ignore", it can be either:

  • skip ignored files, or
  • skip the ignore process itself.

the actual meaning is the second one. the no_ignore value was used inconsistently between the main logic and the dump-inputs logic.

related to #1767

the use of `no_ignore` was missing a `!` before being passed as
skip_ignored. i figure there was some confusion about the meaning of
"no ignore", it can be either:

- skip ignored files, or
- skip the ignore process itself.

the actual meaning is the second one. the no_ignore value was used
inconsistently between the main logic and the dump-inputs logic.

related to https://www.github.com/lycheeverse/lychee/issues/1767
@thomas-zahner
Copy link
Copy Markdown
Member

Thanks for catching that. Doesn't this fully resolve #1767? Did you still observe any notable differences? In the issue I only mentioned the ignore inconsistency. 😅 I should probably double check to see if I spot any other problems.

@katrinafyi
Copy link
Copy Markdown
Member Author

Maybe! It seemed like you and mre had already covered most of the issues, so I didn't look very hard for other differences. But nothing else stood out to me.

@thomas-zahner
Copy link
Copy Markdown
Member

Coincidentally, while resolving clippy suggestions (too many function parameters) in #1891 I extracted the three boolean flags into a Skip struct. While, doing so I also solved this bug. I now cherry-picked your test commit of this PR.

Could you take a look at 96eae64? If you agree with the changes I would like to close this PR in favour of #1891. Reviews of my PR are welcome.

@thomas-zahner thomas-zahner merged commit 7b6425e into lycheeverse:master Nov 1, 2025
6 checks passed
@mre mre mentioned this pull request Nov 1, 2025
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