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

Customizing main RSS path, as well as Atom and RSS feed extensions #3042

Merged
merged 8 commits into from Apr 15, 2018

Conversation

@felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Apr 15, 2018

Fixes #3041

nikola/nikola.py Outdated
@@ -1117,6 +1122,7 @@ def _set_global_context_from_config(self):
self._GLOBAL_CONTEXT['generate_atom'] = self.config.get('GENERATE_ATOM')
self._GLOBAL_CONTEXT['generate_rss'] = self.config.get('GENERATE_RSS')
self._GLOBAL_CONTEXT['rss_path'] = self.config.get('RSS_PATH')
self._GLOBAL_CONTEXT['rss_filename_base'] = self.config.get('RSS_FILENAME_BASE')
Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

*_extension too. Or otherwise make them cause full rebuilds. (We should have a facility for that, that isn’t global context.)

Copy link
Contributor Author

@felixfontein felixfontein Apr 15, 2018

Fixed. I also added documentation for them.

@@ -1059,6 +1066,9 @@ MARKDOWN_EXTENSIONS = ['markdown.extensions.fenced_code', 'markdown.extensions.c
# between each other. Old Atom feeds with no changes are marked as archived.
# GENERATE_ATOM = False

# Extension for Atom feed files
# FEED_EXTENSION = ".atom"
Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

ATOM_EXTENSION

Copy link
Contributor Author

@felixfontein felixfontein Apr 15, 2018

Ok.

Copy link
Contributor Author

@felixfontein felixfontein Apr 15, 2018

And now fixed.

@@ -233,7 +233,7 @@ def _postprocess_path(self, path, lang, append_index='auto', dest_type='page', p
# Forcing extension for Atom feeds and RSS feeds
force_extension = None
if dest_type == 'feed':
force_extension = '.atom'
force_extension = self.site.config['ATOM_EXTENSION']
elif dest_type == 'rss':
force_extension = '.xml'
Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

Shouldn’t this be be RSS_EXTENSION? And what about the ['rss'] on line 243 (gh won’t let me comment there)?

Copy link
Contributor Author

@felixfontein felixfontein Apr 15, 2018

No, since 'feed' is for Atom feeds ('rss' is for RSS feeds). But it looks like I forgot to change .xml two lines below...
And yes, the ['rss'] in line 243 also needs a change.

Copy link
Member

@Kwpolska Kwpolska Apr 15, 2018

The comment was (supposed to be) on line 238.

Copy link
Contributor Author

@felixfontein felixfontein Apr 15, 2018

Ah, ok :)

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Apr 15, 2018

Looks good. @felixfontein, you may merge if all of this was tested, and if the test suite passes.

@felixfontein
Copy link
Contributor Author

@felixfontein felixfontein commented Apr 15, 2018

The default settings work fine (i.e. my blogs compile without changes, except updated RSS/Atom feed timestamps).

@Kwpolska Kwpolska merged commit 75a1f70 into master Apr 15, 2018
1 of 3 checks passed
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Apr 15, 2018

A quick test of non-default settings confirms they work. I’ll merge, and if tests fail (they shouldn’t, py3.3 passed), we’ll fix them.

1 similar comment
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Apr 15, 2018

A quick test of non-default settings confirms they work. I’ll merge, and if tests fail (they shouldn’t, py3.3 passed), we’ll fix them.

@Kwpolska Kwpolska deleted the fix-3041 branch Apr 15, 2018
@felixfontein
Copy link
Contributor Author

@felixfontein felixfontein commented Apr 15, 2018

I was just about to write that the non-default settings seem to work as well :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants