Allow leading spaces for reference links #366
Conversation
Some of the sites that get pulled in use a mix of inline and reference links. Reference links are expected to have the format of the link reference name in brackets, starting at the beginning of the line, followed by a colon and then the linked target. Some of the external sites have been found to have some variation on this format though. While markdown references show the format as above, it does work when these link references have leading spacing before the opening bracket. This has caused [broken slack links](kubernetes/community#6934), as one example. This change updates the regex for identifying these reference links in the get-content script to allow for leading spaces. Signed-off-by: Sean McGinnis <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: stmcginnis The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @stmcginnis! |
k8s-ci-robot
added
the
size/XS
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
There was a problem hiding this comment.
Hm..while we don't have a tests for it - I did a manual check and it seems to break other links =/
here is live: https://www.kubernetes.dev/docs/guide/#community-expectations-and-roles
here is a preview: https://deploy-preview-366--kubernetes-contributor.netlify.app/docs/guide/#community-expectations-and-roles
The 'fuzzyness' of markdown and the regexes have been the bane of the ingestion script -_-
There have been a few attempts to rewrite everything in go and use a native markdown parser, but those have also had other issues.
|
Hmm... there are definitely parts where this is not correct. I can see some cases where there are example markdown comments in docstrings that could be problematic here. There's likely other conditions. It looks like maybe the safest thing for now would be to just enforce that legitimate link references do not have leading characters. I can try to watch out for those and help catch any incoming issues in reviews. In the meantime, I'll watch #93 and see if there is any progress there. If I have a good time block to focus on it, I may take that over and give it one more shot. Currently assigned, but that was quite awhile ago and there hasn't been any progress. So likely they didn't realize the amount of work they were signing up for with that one. :D Will close this for now since it definitely is the best approach. Thanks for taking a look! |
Some of the sites that get pulled in use a mix of inline and reference links. Reference links are expected to have the format of the link reference name in brackets, starting at the beginning of the line, followed by a colon and then the linked target.
Some of the external sites have been found to have some variation on this format though. While markdown references show the format as above, it does work when these link references have leading spacing before the opening bracket. This has caused broken slack links, as one example.
This change updates the regex for identifying these reference links in the get-content script to allow for leading spaces.