-
-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add support for custom file extensions in link checking. #1559
base: master
Are you sure you want to change the base?
Conversation
This adds support for overwriting extensions: ``` lychee . --extensions md,html,txt,json,yaml ``` The above would only check these extensions. This was enabled by moving to `ignore` (#1500 by @thomas-zahner). Fixes #410
@@ -314,7 +314,7 @@ async fn run(opts: &LycheeOptions) -> Result<i32> { | |||
collector | |||
}; | |||
|
|||
let requests = collector.collect_links(inputs); | |||
let requests = collector.collect_links_with_ext(inputs, opts.config.extensions.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd actually prefer collect_links
over collect_links_with_ext
because the latter sounds like the link itself is checked for some extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point. I can switch it around. So what should the method that takes a list of extensions be called?
pub fn collect_links_with_ext( | ||
self, | ||
inputs: Vec<Input>, | ||
extensions: Vec<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename extensions
to file_extensions
or wrap Vec<String>
into a newtype named FileExtensions
? Or maybe we could use the type that ripgrep uses - the one that is created with TypesBuilder::new()
?
@mre looks good overall. Does this mean that non Markdown/HTML files are no longer checked when the new argument isn't specified? |
Yeah, good point. I think plaintext is missing to be compatible with what we had before. That's the only missing format, or? Was considering to extend it to other formats like JSON or YAML as well. |
Co-authored-by: Thomas Zahner <[email protected]>
This adds support for overwriting extensions:
The above would only check these extensions.
This was enabled by moving to
ignore
(#1500 by @thomas-zahner).I'm not 100% convinced about the design yet. Feedback welcome!
Guess we should use whatever
ignore::types::Types
for file extension matching as it's well-maintained and more exhaustive. Switching this would remove some custom code we have inFileType
.Fixes #410