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

Pelican style Markdown metadata (Issue #1923) #2805

Merged
merged 33 commits into from Jun 2, 2017
Merged

Pelican style Markdown metadata (Issue #1923) #2805

merged 33 commits into from Jun 2, 2017

Conversation

@ralsina
Copy link
Member

@ralsina ralsina commented May 28, 2017

This uses the markdown "meta" extension to read metadata in a manner compatible with Pelican (Issue #1923)

It still presents a few problems:

  • Performance: file is read and converted twice.
  • This doesn't really support translated sources, same problem is present in ipynb plugin, because it uses
    source_path instead of translated_source_path. OTOH, translated_source_path doesn't work yet at this stage :-P
if lang is None:
lang = LocaleBorg().current_lang
# FIXME: what about translations????
source = post.source_path
Copy link
Member

@Kwpolska Kwpolska May 29, 2017

Choose a reason for hiding this comment

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

post.translated_source_path(lang)

Also, note that Pelican uses Title-Cased metadata fields, and has some different names.

Loading

Copy link
Member Author

@ralsina ralsina May 29, 2017

Choose a reason for hiding this comment

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

translated_source_path sort of crashes, I need to debug that. Code in ipynb plugin has the same problem.

I'll probably lower-case the keys then.

Loading

@ralsina
Copy link
Member Author

@ralsina ralsina commented May 29, 2017

From invariance results, it seems this is taking priority over actual explicit metadata (at least for title) which is not good, I think.

Loading

lang = LocaleBorg().current_lang
source = post.translated_source_path(lang)

with io.open(source, 'r', encoding='utf-8') as inf:
Copy link
Member

@Kwpolska Kwpolska May 30, 2017

Choose a reason for hiding this comment

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

There should be a way to globally disable this.

Loading

Copy link
Member Author

@ralsina ralsina May 30, 2017

Choose a reason for hiding this comment

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

Good point.

Loading

title: How to make money
slug: how-to-make-money
date: 2012-09-15 19:52:05 UTC

Copy link
Member

@Kwpolska Kwpolska May 30, 2017

Choose a reason for hiding this comment

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

needs docs for reST thing (here and in changelog)

Loading

for lang in self.translations:
if os.path.isfile(get_translation_candidate(self.config, self.source_path, lang)):
self.translated_to.add(lang)
Copy link
Member

@Kwpolska Kwpolska May 30, 2017

Choose a reason for hiding this comment

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

Perhaps we should add a fallback to get_translation_candiate if self.translated_to is empty? There also seems to be a lang != self.default_lang case that will return the source path (shouldn’t it be flipped?)

Loading

Copy link
Member Author

@ralsina ralsina May 30, 2017

Choose a reason for hiding this comment

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

Not really sure. I'll try to take a harder look.

Loading

source = post.translated_source_path(lang)

with io.open(source, 'r', encoding='utf-8') as inf:
document = docutils.core.publish_doctree(
Copy link
Member

@Kwpolska Kwpolska May 30, 2017

Choose a reason for hiding this comment

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

I’m not a reST expert, but perhaps there is a way to make this reusable and store it for compile_string?

Also, we might need to remove field lists from the output, if metadata is imported.

Loading

Copy link
Member Author

@ralsina ralsina May 30, 2017

Choose a reason for hiding this comment

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

Yeah to both things. We can probably keep the doctree somewhere in the Post object and reuse it if it's there.

Same about markdown, actually.

Loading

.gitignore Outdated
@@ -195,5 +195,5 @@ Session.vim
*~

# Demo for Bagguette
Copy link
Member

@Kwpolska Kwpolska May 30, 2017

Choose a reason for hiding this comment

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

this should be at the top, before the gitignore.io template starts

Loading

Copy link
Member Author

@ralsina ralsina May 30, 2017

Choose a reason for hiding this comment

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

Ok, changing it.

Loading

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented May 30, 2017

-<meta property="article:published_time" content="2012-03-30T23:00:00-03:00">
+<meta property="article:published_time" content="2013-12-31T23:59:59Z">

Somehow, the .. date: from Nikola comment metadata is ignored and replaced by the invariant date (over here).

Loading

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented May 30, 2017

Sites using post-list crash:

  File "/Users/kwpolska/git/nikola/nikola/plugins/compile/rest/post_list.py", line 181, in run
    filename = self.state.document.settings._nikola_source_path
AttributeError: 'Values' object has no attribute '_nikola_source_path'

Possible fixes: set that attribute in the metadata thing; disable directives in the metadata thing; make that line failsafe. (@ralsina, you decide)

Loading

Copy link
Member

@Kwpolska Kwpolska left a comment

LGTM, but you may want to squash before merging.

Loading

CHANGES.txt Outdated
USE_REST_DOCINFO_METADATA option, defaulting to False (Issue #1923)
* New HIDE_REST_DOCINFO option, defaulting to False.
* Support for Markdown Meta extension for Pelican
compatibility (Issue #1923)
Copy link
Member

@Kwpolska Kwpolska Jun 2, 2017

Choose a reason for hiding this comment

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

add note about the needed settings

Loading

@@ -604,6 +604,7 @@ def __init__(self, **config):
'USE_BUNDLES': True,
'USE_CDN': False,
'USE_CDN_WARNING': True,
'USE_REST_DOCINFO_METADATA': False,
Copy link
Member

@Kwpolska Kwpolska Jun 2, 2017

Choose a reason for hiding this comment

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

'HIDE_REST_DOCINFO': False,

Loading

.gitignore Outdated
@@ -14,6 +17,8 @@ output/

# All of .idea should be ignored
.idea/
.vscode/
Copy link
Member

@Kwpolska Kwpolska Jun 2, 2017

Choose a reason for hiding this comment

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

Oh, someone’s trying out a new editor!

Loading

Copy link
Member Author

@ralsina ralsina Jun 2, 2017

Choose a reason for hiding this comment

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

And I really like it! except the git "support" which is garbage.

Loading

@ralsina ralsina changed the title (WIP) Pelican style Markdown metadata (Issue #1923) Pelican style Markdown metadata (Issue #1923) Jun 2, 2017
@ralsina ralsina merged commit f1dbc21 into master Jun 2, 2017
0 of 4 checks passed
Loading
@ralsina ralsina deleted the md-metadata branch Jun 2, 2017
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