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

Implements --exclude-file #306

Merged
merged 6 commits into from
Sep 3, 2021
Merged

Implements --exclude-file #306

merged 6 commits into from
Sep 3, 2021

Conversation

dblock
Copy link
Contributor

@dblock dblock commented Sep 1, 2021

Implemented #302.

This is my first rust code, so talk to me like a 2yo :)

lychee-bin/src/main.rs Outdated Show resolved Hide resolved
@lebensterben
Copy link
Member

I think your original proposal of .lycheeignore is great.
We should make it default.

And this also allows the possibility for per-directory exclusion list.

@dblock
Copy link
Contributor Author

dblock commented Sep 1, 2021

I think your original proposal of .lycheeignore is great.
We should make it default.

And this also allows the possibility for per-directory exclusion list.

It's a new construct for Lychee, so I think we should think about it more. The thing with things like .gitignore is that git walks the parent path to find the closest .gitignore until it reaches the git root. This won't be possible here, so maybe it's too much magic?

One way to implement that would be to default --exclude-file to .lycheeexclude if that exists. Can be built on top of this PR.

@lebensterben
Copy link
Member

The thing with things like .gitignore is that git walks the parent path to find the closest .gitignore until it reaches the git root.

lychee supports 'glob'.
so you can do something like lychee ~/notes/**/*.md.
when expanding the glob pattern it's already walking the directories.

lychee-bin/src/main.rs Outdated Show resolved Hide resolved
@mre
Copy link
Member

mre commented Sep 1, 2021

Thanks for the PR.
I also like the idea of a .lycheeignore. In contrast to the .gitignore, a .dockerignore will only be parsed from the build context (usually the current directory), so I would not be too concerned about the differences between the implementations as long as we document the behavior and add tests.

As you both mentioned it could be a nice addition in the future.

@mre mre mentioned this pull request Sep 3, 2021

/// Test excludes
#[test]
fn test_exclude_wildcard() -> Result<()> {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding these additional exclude tests and increasing the test coverage. ❤️

@mre mre mentioned this pull request Sep 3, 2021
3 tasks
@mre
Copy link
Member

mre commented Sep 3, 2021

I'll merge this now. Expect a new release soon. Created a ticket for discussing the future addition of .lycheeignore at #308.
Thanks for first (but hopefully not last) your contribution @dblock. ⭐ And welcome to the Rust community! 🦀

@mre mre merged commit 4b53776 into lycheeverse:master Sep 3, 2021
@dblock dblock mentioned this pull request Sep 3, 2021
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.

None yet

3 participants