Support for custom HTTP headers

Michael-F-Bryan / mdbook-linkcheck

A backend for `mdbook` which will check your links for you.

https://michael-f-bryan.github.io/mdbook-linkcheck/

MIT License

144 stars 29 forks source link

Support for custom HTTP headers #24

Closed CreepySkeleton closed 4 years ago

CreepySkeleton commented 4 years ago

Closes #22

What is implemented

[http-headers]
#regexp = [headers]
"https?://" = ["Accept: text/html", "..."]

Bugs fixed

Old code didn't compare the exclude array properly, for example, if one was a prefix of the another, they would be considered equal (hint: Iterator::zip is short-cutting 😄 ). This is fixed.

What is not implemented

Environment variables interpolation. I agree this is pretty easy to implement, but I'd like to clarify the syntax. $ + ident? maybe also \$ escape...

Further improvements

In my opinion, it would be better to use some wrapper in place of Regex, implementing string-based Hash, Serialize, Deserialize, PartialEq for it. It would greatly simplify certain parts of the code, but that would be a breaking change since you'd already put a Regex in the public API (the exclude array).

TODO

Tests (how should they look like?) and documentation.

Michael-F-Bryan commented 4 years ago

I think we can leave environment variable interpolation as a job for later. Maybe create an issue for it and if people find it necessary it can be implemented on demand... Thoughts?

CreepySkeleton commented 4 years ago

If the syntax is simple $IDENT than I could implement it in no-brainer mode 😄 , not a big burden. But if you want to support ${IDENT} - well, this is where things get complicated...

CreepySkeleton commented 4 years ago

There we go! Sorry for the delay, my spare time suddenly disappeared two days ago :(

I implemented the env interpolation as $IDENT plus \\ and \$ escapes.

Also, I performed a little refactoring, replacing Regex with a wrapper of our on. This does break your public API since the type is different, but I believe this is justifiable change because:

This way the code is much cleaner (IMHO).
Nobody seems to depend on your lib anyway, no offense.

Plus docs and tests.

Michael-F-Bryan commented 4 years ago

Nobody seems to depend on your lib anyway, no offense.

Lol, none taken! It's an executable, so the command-line arguments and configuration part of the public interface but the crate should be considered unstable internals used by the binary.

The entire crate is very specific to a mdbook application, so I doubt people would want to depend on it anyway 😋

Michael-F-Bryan commented 4 years ago

Thanks for all the hard work @CreepySkeleton! I've merged this PR and will make a release the day after tomorrow when I'm back in civilisation.