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

fix: do not fail on empty # and #top fragments #1609

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix: do not fail on empty # and #top fragments
The empty "#" and "#top" fragments are always valid without related HTML element. Browser will scroll to the top of the page. Hence lychee must not fail on those.

Credits go to @thiru-appitap for initial attempt and helping to find missing parts of the implementation.

Solves: #1599

Signed-off-by: MichaIng <micha@dietpi.com>
  • Loading branch information
MichaIng committed Jan 13, 2025
commit 17bf32c5987b991abdd523cf1b0e8fed92a21af0
2 changes: 2 additions & 0 deletions fixtures/fragments/file.html
Original file line number Diff line number Diff line change
@@ -22,6 +22,8 @@
<a href="#Upper-ÄÖö">back to Upper-ÄÖö</a><br>
<a href="#Upper-%C3%84%C3%96%C3%B6">back to öüä encoded</a><br>
<a href="#in-the-end">doesn't exist</a><br>
<a href="#">To the top</a><br>
<a href="#top">To the top alt</a><br>
</section>
</body>
</html>
8 changes: 8 additions & 0 deletions fixtures/fragments/file1.md
Original file line number Diff line number Diff line change
@@ -54,4 +54,12 @@ Therefore we put the test into a code block for now to prevent false positives.
[Link to umlauts wrong case](#fünf-sÜße-Äpfel)
[Link to umlauts with percent encoding](#f%C3%BCnf-s%C3%BC%C3%9Fe-%C3%A4pfel)

# To top fragments

The empty "#" and "#top" fragments are always valid
without related HTML element. Browser will scroll to the top of the page.

[Link to top of file2](file2.md#)
[Alternative link to top of file2](file2.md#top)

##### Lets wear a hat: être
6 changes: 4 additions & 2 deletions lychee-bin/tests/cli.rs
Original file line number Diff line number Diff line change
@@ -1834,8 +1834,10 @@ mod cli {
.stderr(contains(
"fixtures/fragments/file1.md#kebab-case-fragment-1",
))
.stdout(contains("21 Total"))
.stdout(contains("17 OK"))
.stderr(contains("fixtures/fragments/file.html#top"))
.stderr(contains("fixtures/fragments/file2.md#top"))
.stdout(contains("25 Total"))
.stdout(contains("21 OK"))
// 4 failures because of missing fragments
.stdout(contains("4 Errors"));
}
8 changes: 6 additions & 2 deletions lychee-lib/src/utils/fragment_checker.rs
Original file line number Diff line number Diff line change
@@ -39,14 +39,18 @@ impl FragmentChecker {

/// Checks if the given path contains the given fragment.
///
/// Returns false, if there is a fragment in the link and the path is to a
/// Markdown file, which doesn't contain the given fragment.
/// Returns false, if there is a fragment in the link which is not empty or "top"
/// and the path is to a Markdown file, which doesn't contain the given fragment.
/// (Empty # and #top fragments are always valid, triggering the browser to scroll to top.)
///
/// In all other cases, returns true.
pub(crate) async fn check(&self, path: &Path, url: &Url) -> Result<bool> {
let Some(fragment) = url.fragment() else {
return Ok(true);
};
if fragment.is_empty() || fragment.eq_ignore_ascii_case("top") {
return Ok(true);
};
let mut fragment_decoded = percent_decode_str(fragment).decode_utf8()?;
let url_without_frag = Self::remove_fragment(url.clone());

Loading