Closed KyleKing closed 9 months ago
Update: so the problem seems to be that with the latest versions of mdformat and/or linkify, the backslash is splitting the URL into two. Using a different character, such as %
is encoded, but that breaks the CLI test because it produces different HTML.
My guess is that the backslash was a nice hack at the time to verify that the content was being formatted, but now that there have been upstream changes, that hack no longer works. Walking through the relevant logic, the href
only includes the first half of the URL:
# Input: `www.python.org/autolink\extension`
(Pdb) autolink_url = node.attrs["href"]
(Pdb) autolink_url
'http://www.python.org/autolink'
(Pdb) node.children[0].content
'www.python.org/autolink'
(Pdb) autolink_url.split(":", maxsplit=1)[1]
'//www.python.org/autolink'
# which all works expected other than missing the backslash
I'm not sure if there is a use-case where a user shouldn't manually fix the URL containing unencoded characters and there haven't been any bugs reports, so I think this is safe to merge
CI failed because of a time out for one of the combinations, but there was no clear issue (the steps are all about 1 minute). Could be a bug related to how awaiting approval works?
build (3.11, macos-latest) The job running on runner GitHub Actions 6 has exceeded the maximum execution time of 360 minutes.
Testing intermediary Python versions shouldn't be necessary, so I slimmed down the matrix to just the min and max supported Python version, which is currently passing: https://github.com/KyleKing/mdformat-gfm/actions/runs/7495323106 (all: https://github.com/KyleKing/mdformat-gfm/actions)
On a side-note, I tried bumping the upper Python version to 3.12, but ran into issues installing pre-commit (in particular the pyyaml dependency) on Windows with Python 3.12 (one alternative is to change how pre-commit is installed, e.g. just when used, but that's out of scope)
Sorry for being unresponsive.
Thanks! The effort is much appreciated. I wanted to make sure that the upstream changes do not change Markdown AST. It seems that isn't the case.
Previously www.python.org/autolink\extension
was interpreted as an autolink (by linkify-it), but now it isn't. My interpretation of the GFM spec is that it shouldn't be (the backslash and whatever follows, that is). If prefixed by http://
it should be a valid link though, I've updated the test accordingly and named it better.
CI seems to work fine for me so I kept it as is.
When the URL scheme is present, the URL is recognized and parsed, which may have once worked on older versions of
linkify
, but is not working today. There aren't logs to confirm, but the historic CI failures go back to 2021, but the logs have expired: https://github.com/hukkin/mdformat-gfm/actions/runs/3567981113?pr=20I think it's reasonable to make this change because the test is verifying that the
autolink
extension is run and not how the extension is implemented