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

Make self feed url absolute #29

Closed
wants to merge 3 commits into from
Closed

Make self feed url absolute #29

wants to merge 3 commits into from

Conversation

okelet
Copy link

@okelet okelet commented Mar 4, 2016

@tomap
Copy link
Contributor

tomap commented Jul 9, 2019

Indeed, I saw the same warning when trying to validate the feed

@puremana
Copy link

Is there a reason this hasn't been merged? I'm getting the same warning. I understand it's only a warning and has no impact, but if it can be fixed to prevent further confusion I think that would be a good idea.

@darekkay
Copy link
Contributor

This PR has been open for 4 years now. I'd be happy to contribute (and e.g. resolve the merge conflicts), but I fear the work will be for nothing 🙁

@SukkaW
Copy link
Member

SukkaW commented May 23, 2020

@darekkay It will be appreciated if you going to address the PR again.

Also, the code here might help: https://github.com/hexojs/hexo-util/blob/master/lib/full_url_for.js

darekkay added a commit to darekkay/hexo-generator-feed that referenced this pull request May 26, 2020
This PR is an update to hexojs#29.

This supposed to be a small PR, but I've encountered some issues regarding the base URL.

The `[full_url_for](https://github.com/hexojs/hexo-util/blob/master/lib/full_url_for.js#L27)` function expects the `config.url` not to end with a forward slash. Otherwise it will create a double slash URL, e.g. `http://example.com//atom.xml` (notice that a double slash replacement happens only _after_ the base URL).

I was wondering how the icon feature (hexojs#102) can be using `full_url_for` with the current code and not run into any double slash issues. The answer is: it doesn't handle this case. The [unit tests](https://github.com/hexojs/hexo-generator-feed/pull/102/files#diff-910eb6f57886ca16c136101fb1699231R240) are simply running against a custom base URL without an ending slash.

I've cleaned up the tests:
- removed the ending slash from the base URL
- adjusted and added new tests to handle subdirectories correctly (as specified in the [hexo documentation](https://hexo.io/docs/configuration.html#URL))
- hard-coded expected values - it makes a test more reliable and readable
@darekkay
Copy link
Contributor

Follow-up PR: #143

darekkay added a commit to darekkay/hexo-generator-feed that referenced this pull request May 26, 2020
This PR is an update to hexojs#29.

This supposed to be a small PR, but I've encountered some issues regarding the base URL.

The `[full_url_for](https://github.com/hexojs/hexo-util/blob/master/lib/full_url_for.js#L27)` function expects the `config.url` not to end with a forward slash. Otherwise it will create a double slash URL, e.g. `http://example.com//atom.xml` (notice that a double slash replacement happens only _after_ the base URL).

I was wondering how the icon feature (hexojs#102) can be using `full_url_for` with the current code and not run into any double slash issues. The answer is: it doesn't handle this case. The [unit tests](https://github.com/hexojs/hexo-generator-feed/pull/102/files#diff-910eb6f57886ca16c136101fb1699231R240) are simply running against a custom base URL without an ending slash.

I've cleaned up the tests:
- removed the ending slash from the base URL
- adjusted and added new tests to handle subdirectories correctly (as specified in the [hexo documentation](https://hexo.io/docs/configuration.html#URL))
- hard-coded expected values - it makes a test more reliable and readable
SukkaW pushed a commit that referenced this pull request Jun 1, 2020
This PR is an update to #29.

This supposed to be a small PR, but I've encountered some issues regarding the base URL.

The `[full_url_for](https://github.com/hexojs/hexo-util/blob/master/lib/full_url_for.js#L27)` function expects the `config.url` not to end with a forward slash. Otherwise it will create a double slash URL, e.g. `http://example.com//atom.xml` (notice that a double slash replacement happens only _after_ the base URL).

I was wondering how the icon feature (#102) can be using `full_url_for` with the current code and not run into any double slash issues. The answer is: it doesn't handle this case. The [unit tests](https://github.com/hexojs/hexo-generator-feed/pull/102/files#diff-910eb6f57886ca16c136101fb1699231R240) are simply running against a custom base URL without an ending slash.

I've cleaned up the tests:
- removed the ending slash from the base URL
- adjusted and added new tests to handle subdirectories correctly (as specified in the [hexo documentation](https://hexo.io/docs/configuration.html#URL))
- hard-coded expected values - it makes a test more reliable and readable
@stevenjoezhang
Copy link
Member

Thanks for your contribution!

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.

6 participants