Skip to content

Conversation

@kemingy
Copy link
Contributor

@kemingy kemingy commented Apr 11, 2025

@kemingy
Copy link
Contributor Author

kemingy commented Apr 11, 2025

@mre
Copy link
Member

mre commented May 9, 2025

Nice pull request! Only had one improvement idea.

@kemingy kemingy requested a review from mre May 11, 2025 15:47
Copy link
Member

@mre mre left a comment

Choose a reason for hiding this comment

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

Looks good to me!
We should add a test or two to make sure it works. I guess the easiest would be an integration test in lychee-bin/tests/cli.rs. But I wouldn't mind merging this already, in case you won't find the time to add it. Just let me know. 😉

@kemingy
Copy link
Contributor Author

kemingy commented May 12, 2025

Looks good to me! We should add a test or two to make sure it works. I guess the easiest would be an integration test in lychee-bin/tests/cli.rs. But I wouldn't mind merging this already, in case you won't find the time to add it. Just let me know. 😉

Added to the test cases for both markdown and HTML.

TIL, GitHub HTML has a "user-content-" prefix for fragments added in the README file.

@mre
Copy link
Member

mre commented May 12, 2025

Yes, great! One final rebase, and we should be good to go. 😄

Signed-off-by: Keming <[email protected]>
@kemingy
Copy link
Contributor Author

kemingy commented May 13, 2025

Yes, great! One final rebase, and we should be good to go. 😄

Rebased. PTAL

@kemingy kemingy requested a review from mre May 13, 2025 10:42
@mre mre merged commit 1ed357f into lycheeverse:master May 13, 2025
6 checks passed
@mre
Copy link
Member

mre commented May 13, 2025

Fantastic! Thank you.

This was referenced May 13, 2025
@kemingy kemingy deleted the web_fragment branch May 14, 2025 00:05
@honzajavorek
Copy link

This has introduced several broken links on my website. It's because they point to websites where the fragment is handled by JavaScript, so the link does work, but Lychee can't see it. It would be awesome if I could add something like "ignore fragments checking for these few links where I manually verified it's actually okay" option to my TOML 🙏 Without it I'll have to turn off checking fragments altogether, and that would be a shame.

@honzajavorek
Copy link

As an example, https://github.com/lycheeverse/lychee#readme works in the browser, but Lychee will deem it as broken fragment. That's because GitHub uses React nonsense to render the page.

@kemingy
Copy link
Contributor Author

kemingy commented May 15, 2025

As an example, https://github.com/lycheeverse/lychee#readme works in the browser, but Lychee will deem it as broken fragment. That's because GitHub uses React nonsense to render the page.

I think we can add some special logic to handle the GitHub Markdown fragments. (lychee already has some special logic for GitHub in the code)

We could detect if the URL contains "github.com" then try the "user-content-" prefix for the fragments.

I have checked that GitHub gist also follows this rules.

What do you think? @mre

@honzajavorek
Copy link

honzajavorek commented May 15, 2025 via email

@mre
Copy link
Member

mre commented May 15, 2025

It would be awesome if I could add something like "ignore fragments checking for these few links where I manually verified it's actually okay" option to my TOML

I guess you can?

❯❯❯ echo 'https://github.com/lycheeverse/lychee#readme' | lychee -vvv --exclude 'https://github.com/lycheeverse/lychee#' -
[EXCLUDED] https://github.com/lycheeverse/lychee#readme

🔍 1 Total (in 0s) ✅ 0 OK 🚫 0 Errors 👻 1 Excluded

The trick is to add a # at the end of the site you want to exclude.

Also, at least for me, your example link does work:

❯❯❯ echo 'https://github.com/lycheeverse/lychee#readme' | lychee -vvv -
     [200] https://github.com/lycheeverse/lychee#readme

🔍 1 Total (in 0s) ✅ 1 OK 🚫 0 Errors

Can somebody double-check?

If it turns out to be an issue, it would be better to create a new issue referencing this pull request instead.

@honzajavorek
Copy link

I can exclude the links completely, but then they're not checked if they're 404.

Also, at least for me, your example link does work

That's interesting. I didn't have time to dig into this for the past week. I'll get back to it soon and if there's a feature to request, I'll request it as a separate issue. Thanks for Lychee and thanks for replying here 🙏

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.

bug: anchor/fragment detection doesn't appear to work

3 participants