Repository navigation
Fetch links to Markdown files - #14
Conversation
|
I tested on a couple of WAT from (CC-MAIN-20260917025921-20260917055921-0099{7,8,9}.warc.wat.gz), we have around 70 new links per WAT file. |
sebastian-nagel
left a comment
sebastian-nagel
left a comment
There was a problem hiding this comment.
Thanks, @lfoppiano! Looks excellent!
Nothing to criticize. Only one suggestions to further improve the PR: remove the condition restriction the addition of HTTP header links to only Markdown documents. Include also canonical links, alternate language pages, feeds - everything accepted by addOutLinks(...). See inline comments.
| if (httpHeaderNames == null) { | ||
| return; | ||
| } | ||
| JSONArray links = new JSONArray(); |
There was a problem hiding this comment.
Excellent idea to fill the HTTP header links into a JSON structure ...
| } | ||
| } | ||
| if (links.length() > 0) { | ||
| addOutLinks(context, outLinks, baseUrl, links); |
There was a problem hiding this comment.
... and pass them to the addOutLinks(...).
| link.put(name, val); | ||
| } | ||
| } | ||
| if (link.has("type") && matchesMediaType(link.getString("type"), "text/markdown")) { |
There was a problem hiding this comment.
Since addOutLinks(...) already filters type=text/markdown, why not remove this condition? - and update the Javadoc.
Of course, this would also allow canonical links, alternate language pages, and (if configured) RSS/Atom feeds. But that would be a further improvement! And the rather complex decision which links to accept, would be in a single place (code block). Unwanted links (CSS stylesheets, dns-prefetch, pingbacks, etc.) are reliably filtered out by addOutLinks(...).
| if (extractFeed && link.has("type")) { | ||
| String type = link.getString("type"); | ||
| if ("application/atom+xml".equals(type) || "application/rss+xml".equals(type)) { | ||
| if (matchesMediaType(type, "application/atom+xml") |
|
@sebastian-nagel thanks for the feedback! It was very helpful to learn more about this job. I've updated the code following your suggestions. I also added an additional check to enforce the presence of the |
This PR adds the retrieval of links to markdown files:
LinkLINK@/hrefrel=describedbyis not used, only the type is usedThe type check is strengthen to support encoding suffix.