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 #11: Adapt link regex to exclude anchor links #14

Merged
merged 3 commits into from
Feb 26, 2024

Conversation

sdruskat
Copy link
Contributor

@sdruskat sdruskat commented Nov 24, 2023

First up, thanks for a very helpful plugin!

I've stumbled over #11 just now and am supplying a fix in this PR.

  • Changes the LINK_PATTERN_REGEX to only match links that don't start with a hash by adding [^#] in front of the link_url group.
  • Manually tested locally with a vanilla page built with mkdocs v1.5.3 (mkdocs new ., add external page with anchor links, e.g., this README: {{ external_markdown('https://raw.githubusercontent.com/fefong/markdown_readme/master/README.md', '') }}.

I hope this is acceptable. Please let me know shold you need anything changed, and feelf free to edit yourself.

- Changes the LINK_PATTERN_REGEX to only match links that don't
start with a hash by adding '[^#]' in front of the link_url group.
@sdruskat
Copy link
Contributor Author

Note that this doesn't fix links to anchors on other pages, see #11 (comment).

@sdruskat
Copy link
Contributor Author

This currently needs more work, so returning it to draft mode.

@sdruskat sdruskat marked this pull request as draft November 26, 2023 20:34
* Add test dependencies and document how to install and run tests
* Add regression tests for different types of links
* Fix fire1ce#11 properly: Test for `#` prefix in links
    * Rolls back the broken fix in 8d4b5ee
    *Fixes fire1ce#11 by checking for a hash prefix in closure code
@sdruskat
Copy link
Contributor Author

The proposed fix in 8d4b5ee actually introduced a bug where the first character of the link URL was chopped off, so I rolled back, added tests for a correct fix and implemented it in a8f5bea.

This is now ready for a review. Please let me know should you like anything changed.

@sdruskat sdruskat marked this pull request as ready for review November 27, 2023 09:17
@fire1ce
Copy link
Owner

fire1ce commented Nov 27, 2023

@sdruskat thanks for a PR. l'll try to review it as soon as possible but it may take some time since im busy in the near future. Hope to find some proper time to review the changes.

@sdruskat
Copy link
Contributor Author

@sdruskat thanks for a PR. l'll try to review it as soon as possible but it may take some time since im busy in the near future. Hope to find some proper time to review the changes.

@fire1ce Thanks for the quick note! I know what it's like, appreciate your maintenance of this open source project, and don't place any expectations on you w.r.t. a quick review/merge.

@fire1ce fire1ce merged commit 340bdaa into fire1ce:main Feb 26, 2024
1 check passed
@fire1ce
Copy link
Owner

fire1ce commented Feb 26, 2024

Thanks agan. Sorry im very late.

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.

2 participants