Jekyll: Fix many bugs, add new features #506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markstos
force-pushed
the
rename-posts
branch
2 times, most recently
from
6fc40f9
to
5924fe6
Compare
|
I've also added a fix to #508 to this PR, as it would have conflicted if I'd submitted it independently. |
The Jekyll docs are clear that posts go in "\_posts" (with an underscore): https://jekyllrb.com/docs/structure/ While "posts" (no underscore) may be the the name of an output directory with rendered HTML. Because HTML is also valid as an impout format, importing "posts" is no longer supported to avoid confusion.
|
I implemented support for HTML format content (#507) on this branch, since it was also going to conflict badly if I started that work based on |
|
As I've discovered more importing bugs with easy fixes, this PR can be considered still in progress for now if you don't mind a bigger collection of fixes to review later. I also found a bug in this PR: I used |
I"m not able to find a reference for the complete list of Markdown file extensions that Jekyll supports, but instance uses ".markdown" and I don't seem to have done anything special to enable that.
The location of the drafts folder should be _drafts, not "drafts". Also, support ".markdown" extension in more spots.
HTML posts are detected using an ".html" file extension. Before, the entire contents of Jekyll posts was referred to as "Markdown". This was not completely accurate, as it is a combination of "front matter" plus Markdown. And this not accurate at all ilf the file content was actually html. Now, we use "fileContents" to refer to the whole file. Before, the front matter was being parsed twice, once during metadata processing and a second time for "markdown" processing. Now, frontmatter only gets processed ones, which should be a small speed-up. Before, "process-content.js" was almost exclusively updating HTML and not any other type of content. Now that HTML support has been added, that's the only thing that it does, so it renamed "process-html.js" to be more clear. I'm not sure if Jekyll supports multiple extensions for HTML files like it does for Markdown files. Here only support for ".html" is added. If an unknown file type is detected, an error will be thrown. Fixes TryGhost#507
This is for consistency with how `js-yaml` parses the "date" frontmatter, which will be dealt with in a future commit. Also, fix the inaccurate comment that was trying to explain JavaScript's 0-based dates, but got confused itself in the process.
Unlike quoted dates, the underlying js-yaml library parses these directly into date objects. Fixes TryGhost#511
Linting rules require throwing GhostErrors, which have the web in mind.
The user intended to migrate some blog posts. If no content was found, report an error instead of reporting "Successfully imported zero posts"
First, the intent was to read files in the `_drafts/` folder was well, but the Regex was not matching it. Second, the intent was to match against the path stored in the the zipfile, but that was broken. Confusingly, "entryName" holds just the file name while "zipEntry.entryName" holds the full path. The fix is to switch "entryName" we use. Third, only the bare file name was being reported when skipped. It's more useful to see the full path within the zip, in case the problem is with the folder structure in there. Fixes TryGhost#510
Before, the first tag would have been imported first as the "primary tag". Now, because the Ghost's "primary tag" is the best approximation of Jekyll's "category", we import the "category first".
Fixes cases where <a> tag has no "href" and <img> tag has no "src". Fixes TryGhost#513.
|
I'm finishing pushing bug fixes to this branch for now. I'm currently stuck on a bug where scraping is happening even with |
Before, the code /and/ tests wrongly assumed that Jekyl resolved relative-to-root URLs relative to the blogs "url", but it resolves them to the origin domain. Fixes TryGhost#516
markstos
changed the title
|
Hey @markstos, thank you so much for your efforts and expertise here, it's massively appreciated! All of this has been merged 🙌 |
|
@PaulAdamDavis Thanks for the merge and your maintainership! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
UPDATE: This PR was originally about the narrow scope of fixing the bug below. But I soon ran into a number other bugs and missing features that affected the same parts of the code. It quickly became not feasible to submit separate PRs for them, as many would overlap would conflict, so this PR collects all the fixes I've made. Looking at the individual commits will provide context for each change.
Bug: "Should import _posts, not "posts""
The Jekyll docs are clear that posts go in "_posts" (with an
underscore):
https://jekyllrb.com/docs/structure/
While "posts" (no underscore) may be the the name of an output directory
with rendered HTML.
Because HTML is also valid as an impout format, importing "posts" is
no longer supported to avoid confusion.