fix : add feed icon to rss2.xml #102

sehajyang · 2019-10-30T02:23:33Z

I configured feed.icon in the config.yml file.
but that icon was not exposed when creating the feed xml file.
So, I add in rss2.xml :

<url> {{(url + config.feed.icon) | uriencode}} </ url>

and I see that it works.

I configured `feed.icon` in the `config.yml` file but that icon was not exposed when creating the feed xml file. So, I Add in rss2.xml `<url> {{(url + config.feed.icon) | uriencode}} </ url>` and I see that it works.

coveralls · 2019-10-30T02:24:54Z

Coverage remained the same at 100.0% when pulling 417898d on sehajyang:patch-1 into d169992 on hexojs:master.

SukkaW

After this change, it will become impposible to use third-party url of feed icon.

Please use full_url_for() instead.

sehajyang · 2019-10-30T04:50:16Z

full_url_for() seems to add the blog url as a prefix.
So if it's a third-party url, it seems to be blog url + third-party url. am I right?

SukkaW · 2019-10-30T04:52:24Z

@sehajyang Nope.

https://github.com/hexojs/hexo-util/blob/d9081722c3220e7943f073e84ff3cf10db499a8a/lib/full_url_for.js#L21

sehajyang · 2019-10-30T06:55:07Z

@sukka

I installed hexo-util and added the following code to generator.js:

const all_url_for = require ('hexo-util'). all_url_for.bind (hexo);

var icon;
if (config.email) {
    icon = gravatar (config.email)
} else {
    icon = all_url_for (config.feed.icon)
}

However, there was an error where config could not be found in all_url_for ().
Also bind (hexo); didn't work. I tried a few ways to solve this problem but it failed.
How can i use all_url_for?

SukkaW · 2019-10-30T07:14:43Z

@sehajyang Try this?

First, require full_url_for at the beginning:

hexo-generator-feed/lib/generator.js

Line 7 in d169992

const { encodeURL, gravatar } = require('hexo-util');

 const { encodeURL, gravatar, full_url_for } = require('hexo-util');

Then change L38:

hexo-generator-feed/lib/generator.js

Line 38 in d169992

if (feedConfig.icon) icon = url + encodeURI(feedConfig.icon);

to:

if (feedConfig.icon) icon = encodeURI(full_utl_for.call(this, feedConfig.icon));

You can also include shorter object syntax in this PR:

hexo-generator-feed/lib/generator.js

Lines 41 to 47 in d169992

    
           const xml = template.render({ 
        
             config: config, 
        
             url: url, 
        
             icon: icon, 
        
             posts: posts, 
        
             feed_url: config.root + feedConfig.path 
        
           });

to

 const xml = template.render({ 
   config, 
   url, 
   icon, 
   posts, 
   feed_url: config.root + feedConfig.path 
 });

sehajyang · 2019-10-30T08:06:44Z

@SukkaW
I fixed it as your comment and saw that it works.
Thanks to your comment I was able to solve this issue. Thank you for your help 😊

lib/generator.js

rss2.xml

curbengh · 2019-11-03T00:51:20Z

Unit test is also missing, let me know if you need help creating one.

sehajyang · 2019-11-04T01:20:04Z

@curbengh
Yes I need help. Can you make one?
Also, I modified the part you commented above. Let me know if there's anything I need to change.

rss2.xml

…-feed into patch-1

curbengh · 2019-11-05T01:19:12Z

I just added two tests, for atom only. Simply duplicate them (not replace), change the necessary config to rss2 then change the corresponding expected result.

A brief guide for using cheerio; atom.xml starts with the following value,

<feed xmlns="https://www.w3.org/2005/Atom">
  <title>{{ config.title }}</title>
  <icon>{{ icon }}</icon>

Notice $('feed>icon') corresponds to the <feed> and <icon> elements. The > is a shortcut for .children(); in this case, we're looking for <icon> in the children of <feed>.

Hint: for subsequent levels,

<feed xmlns="https://www.w3.org/2005/Atom">
  <title>{{ config.title }}</title>
  <foo>
    <bar>baz</bar>
  <foo>

$('feed>foo>bar').text()
// baz

atom.xml

This reverts commit 07f8b70.

sehajyang · 2019-11-05T02:05:22Z

@curbengh
I created the rss2 icon's test code with your comments and the atom icon's test code.
Thank you for your help.
Please let me know if there are any errors.

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

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

Add : feed icon to rss2.xml

066aafc

I configured `feed.icon` in the `config.yml` file but that icon was not exposed when creating the feed xml file. So, I Add in rss2.xml `<url> {{(url + config.feed.icon) | uriencode}} </ url>` and I see that it works.

SukkaW requested changes Oct 30, 2019

View reviewed changes

Fix : generator.js, icon url can third-party url

46be47d

sehajyang requested a review from SukkaW October 30, 2019 08:04

SukkaW approved these changes Oct 30, 2019

View reviewed changes

curbengh requested changes Nov 3, 2019

View reviewed changes

lib/generator.js Outdated Show resolved Hide resolved

curbengh requested changes Nov 3, 2019

View reviewed changes

rss2.xml Outdated Show resolved Hide resolved

sehajyang added 2 commits November 4, 2019 09:56

Fix : rss2.xml, add image, link tag for icon

def6183

Fix : del encodeURL() that wrapping full_url_for in generator.js

8ccffee

sehajyang requested a review from curbengh November 4, 2019 05:52

curbengh requested changes Nov 5, 2019

View reviewed changes

rss2.xml Show resolved Hide resolved

curbengh and others added 4 commits November 5, 2019 01:09

test(atom): test icon config

f8dde0c

Fix : add icon exist check to rss2.xml

b58ec71

Fix : atom.xml, added wapper tag for icon

07f8b70

Merge branch 'patch-1' of https://github.com/sehajyang/hexo-generator…

ac11d2d

…-feed into patch-1

curbengh requested changes Nov 5, 2019

View reviewed changes

atom.xml Outdated Show resolved Hide resolved

Revert "Fix : atom.xml, added wapper tag for icon"

e4ea6f6

This reverts commit 07f8b70.

test(rss2) : test rss2 icon config

417898d

sehajyang requested a review from curbengh November 5, 2019 02:42

curbengh approved these changes Nov 5, 2019

View reviewed changes

curbengh merged commit d168026 into hexojs:master Nov 5, 2019

sehajyang deleted the patch-1 branch November 5, 2019 05:11

sehajyang changed the title ~~Add : feed icon to rss2.xml~~ fix : add feed icon to rss2.xml Dec 27, 2019

darekkay mentioned this pull request May 26, 2020

Render an absolute feed URL #143

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix : add feed icon to rss2.xml #102

fix : add feed icon to rss2.xml #102

sehajyang commented Oct 30, 2019 •

edited

Loading

coveralls commented Oct 30, 2019 •

edited

Loading

SukkaW left a comment

sehajyang commented Oct 30, 2019 •

edited

Loading

SukkaW commented Oct 30, 2019

sehajyang commented Oct 30, 2019

SukkaW commented Oct 30, 2019 •

edited

Loading

sehajyang commented Oct 30, 2019 •

edited

Loading

curbengh commented Nov 3, 2019

sehajyang commented Nov 4, 2019 •

edited

Loading

curbengh commented Nov 5, 2019 •

edited

Loading

sehajyang commented Nov 5, 2019 •

edited

Loading

fix : add feed icon to rss2.xml #102

fix : add feed icon to rss2.xml #102

Conversation

sehajyang commented Oct 30, 2019 • edited Loading

coveralls commented Oct 30, 2019 • edited Loading

SukkaW left a comment

Choose a reason for hiding this comment

sehajyang commented Oct 30, 2019 • edited Loading

SukkaW commented Oct 30, 2019

sehajyang commented Oct 30, 2019

SukkaW commented Oct 30, 2019 • edited Loading

sehajyang commented Oct 30, 2019 • edited Loading

curbengh commented Nov 3, 2019

sehajyang commented Nov 4, 2019 • edited Loading

curbengh commented Nov 5, 2019 • edited Loading

sehajyang commented Nov 5, 2019 • edited Loading

sehajyang commented Oct 30, 2019 •

edited

Loading

coveralls commented Oct 30, 2019 •

edited

Loading

sehajyang commented Oct 30, 2019 •

edited

Loading

SukkaW commented Oct 30, 2019 •

edited

Loading

sehajyang commented Oct 30, 2019 •

edited

Loading

sehajyang commented Nov 4, 2019 •

edited

Loading

curbengh commented Nov 5, 2019 •

edited

Loading

sehajyang commented Nov 5, 2019 •

edited

Loading