Skip to content

Add Link header support#301

Merged
ai merged 3 commits intohplush:mainfrom
ashlkv:link-header
Jun 4, 2025
Merged

Add Link header support#301
ai merged 3 commits intohplush:mainfrom
ashlkv:link-header

Conversation

@ashlkv
Copy link
Copy Markdown
Contributor

@ashlkv ashlkv commented Jun 3, 2025

Fixes #27

  • Feed loaders now extract feed urls of their respective types from Link header using the shared utility function
  • Added and updated the tests

Motivation

The Link http header can sometimes contain the link to the feed. This http header is used when the document itself is difficult to modify.

  • Feed loaders look for a feed url of their respective type in the Link http header
  • If found, this link or links take priority over links found in the document's html
  • Works even if the document is not html (should it?)

Comment thread core/loader/utils.ts
.map(a => buildFullURL(a, text.url))
}

/**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very good comment. But we don’t use it in another functions.

Can we keep the old comment-less format in PR?

But I am open to discuss adding more comments (just prefer to have some policy).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's consider adding more comments. It's hard to understand what functions do otherwise, even looking at tests. Especially for newcomers. And especially now without ui.

Comment thread core/test/loader/utils.test.ts Outdated
Comment thread core/loader/utils.ts
Comment thread core/loader/utils.ts Outdated
@ashlkv ashlkv marked this pull request as ready for review June 3, 2025 12:06
@ai ai merged commit e79323f into hplush:main Jun 4, 2025
3 of 4 checks passed
@ai
Copy link
Copy Markdown
Contributor

ai commented Jun 4, 2025

Thanks a lot! Very professional work.

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.

Add Link header support

2 participants