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
Generalize metadata functions and moving them to utils.py #2856
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted a different kind of refactor in #2380, this would kind-of break it IMO.
nikola/utils.py
Outdated
elif not match and result: | ||
return (result[0][0], result[0][1].strip()) | ||
else: | ||
return (None,) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return None, None
— and no parentheses above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine for me. I just copy'n'pasted this code from post.py
and didn't modify anything in this function.
* ``'rest'``: metadata in reST format (the standard Nikola | ||
reST-like metadata format) | ||
""" | ||
if data.startswith('---'): # YAML metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be something to split metadata based on the post object as well, and the post object should know what metadata type it was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be for #2830, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I’ll handle it then.
Yeah, that was a typo: #2830. I’ll go 👍 for merging it, but don’t expect this API to be stable. |
return '', split_result[0] | ||
# ['metadata', '\n\n', 'post content'] | ||
return split_result[0], split_result[-1] | ||
meta, content, _ = split_metadata(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this one. Is codacy maybe confusing the imported utils.split_metadata
with PageCompiler.split_metadata
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be. Ignore that warning.
Sure. I assume nothing in I'm not sure whether I'll use this API though, now that you mentioned #2830... |
nikola/post.py
Outdated
if meta[k] is None: | ||
meta[k] = '' | ||
meta, metadata_type = utils.extract_metadata(file_lines) | ||
if metadata_type == 'yaml': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For both blocks:
if metadata_type in ('yaml', 'toml'):
# Map metadata from other platforms to names Nikola expects (Issue #2817)
map_metadata(meta, metadata_type, config)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
||
|
||
# Moved to global variable to avoid recompilation | ||
# of regex every time re_meta() is called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex fun fact: there’s a cache, for 512 entries if memory serves. It probably doesn’t get recompiled. (Don’t change this back)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I didn't knew about that cache. I don't like the idea of depending on it in such cases where it is obvious that the regex will be used over and over again, though, so yes, I won't change this back :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for reviewing! |
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
* Rename UNSLUGIFY_TITLES → FILE_METADATA_UNSLUGIFY_TITLES Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Initial scaffolding for metadata extractor plugins Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Use correct environment marker syntax Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Style fixes Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Use metadata extractor APIs and plugins Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Fix galleries; add basic split support (temporary) Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Fix compatibility with .meta files Signed-off-by: Chris Warrick <kwpolska@gmail.com> * #2856 was effectively undone Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Make metadata splitting work Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Code cleanups Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Fix tests, add new config_present condition Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Allow whitespace before Nikola-style metadata Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Fix RSS tests Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Add MetadataExtractors documentation and `override` MetaPriority Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Style fixes Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Add tests for metadata extractors Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Turns out we run flake8 on tests, too Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Make write_metadata work with metadata extractors Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Fix #2830 — add MetadataExtractor plugins Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Minor documentation fixes [ci skip] Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Address review by @felixfontein Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Remove useless comment [ci skip] Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Slightly smarter return values for split_metadata_for_text Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Pass site when writing metadata in importers Note this is a minor API change (removes @staticmethod) that needs to be reflected in some importers. cc @felixfontein Signed-off-by: Chris Warrick <kwpolska@gmail.com> * Document potentially confusing split_metadata_from_text behavior Signed-off-by: Chris Warrick <kwpolska@gmail.com>
This makes implementing getnikola/plugins#232 much easier, and separates the metadata extraction from the higher level parts (metadata transformation).