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

Handle complex parsing logic in new parser action #325

Merged
merged 24 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
efb30ac
Extract complex constructor logic to helper method
caendesilva Aug 3, 2022
564a2de
Find the page title in parser
caendesilva Aug 3, 2022
97c14a0
Apply fixes from StyleCI
StyleCIBot Aug 3, 2022
b9d9a51
Delete FindsTitleForDocument.php
caendesilva Aug 3, 2022
4e9245e
Merge branch 'improve-file-parsing' of github.com:hydephp/develop int…
caendesilva Aug 3, 2022
82b9280
Merge branch 'master' into improve-file-parsing
caendesilva Aug 3, 2022
4a88f7c
Use arrayable method
caendesilva Aug 3, 2022
e94e225
Remove trait HasDynamicTitle
caendesilva Aug 3, 2022
576fe25
Merge branch 'master' into improve-file-parsing
caendesilva Aug 3, 2022
f9715f4
Apply fixes from StyleCI
StyleCIBot Aug 3, 2022
82d9c3e
Add backwards compatibility to assign title from constructor frontmatter
caendesilva Aug 3, 2022
9e7803c
Parser now infers title automatically
caendesilva Aug 3, 2022
90436f3
Use the already parsed title
caendesilva Aug 3, 2022
1821529
Update history
caendesilva Aug 3, 2022
db63158
Discover documentation page category in parser
caendesilva Aug 3, 2022
1d456d3
Replace null coalesce with helper method call
caendesilva Aug 3, 2022
2614965
Simplify 'if' with ternary operator
caendesilva Aug 3, 2022
cc15a16
Document code behaviour
caendesilva Aug 3, 2022
ad8a66c
Update code documentation
caendesilva Aug 3, 2022
4dce275
Add some spacing
caendesilva Aug 3, 2022
f3d6a07
Format comments
caendesilva Aug 3, 2022
b715bbb
Replace comment with more descriptive method name
caendesilva Aug 3, 2022
e377f2a
Replace false return value with null
caendesilva Aug 3, 2022
41ff3ef
Apply fixes from StyleCI
StyleCIBot Aug 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ This update refactors the internal page source model parsing. This will likely n
- Blog post front matter no longer includes merged slug
- MarkdownDocument now implements the `Arrayable` interface
- Markdown page models no longer includes the slug merged into the front matter
- All Markdown page models now have the title property inferred when parsing
- internal: The DocumentationPage slug now behaves like other pages, and the basename is produced at runtime, see below
- internal: Refactor search index generator to use route system

Expand All @@ -22,6 +23,7 @@ This update refactors the internal page source model parsing. This will likely n
### Removed
- The PageParserContract interface, and all of its implementations have been removed
- Removed `$localPath` property from DocumentationPage class, see above
- Removed trait HasDynamicTitle

### Fixed
- for any bug fixes.
Expand Down
35 changes: 0 additions & 35 deletions packages/framework/src/Actions/FindsTitleForDocument.php

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function generatePageObject(DocumentationPage $page): object
{
return (object) [
'slug' => $page->slug,
'title' => trim($page->findTitleForDocument()),
'title' => $page->title,
'content' => trim($this->getSearchContentForDocument($page)),
'destination' => $this->getDestinationForSlug($page->slug),
];
Expand Down
57 changes: 54 additions & 3 deletions packages/framework/src/Actions/SourceFileParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
use Hyde\Framework\Concerns\ValidatesExistence;
use Hyde\Framework\Contracts\AbstractMarkdownPage;
use Hyde\Framework\Contracts\PageContract;
use Hyde\Framework\Hyde;
use Hyde\Framework\Models\Pages\BladePage;
use Hyde\Framework\Models\Pages\DocumentationPage;
use Hyde\Framework\Modules\Markdown\MarkdownFileParser;
use Illuminate\Support\Str;

/**
* Parses a source file and returns a new page model instance for it.
Expand All @@ -28,10 +31,15 @@ class SourceFileParser
public function __construct(string $pageClass, string $slug)
{
$this->validateExistence($pageClass, $slug);

$this->slug = $slug;

$this->page = $pageClass === BladePage::class
$this->page = $this->constructBaseModel($pageClass);
$this->constructDynamicData();
}

protected function constructBaseModel(string $pageClass): BladePage|AbstractMarkdownPage
{
return $pageClass === BladePage::class
? $this->parseBladePage()
: $this->parseMarkdownPage($pageClass);
}
Expand All @@ -54,11 +62,54 @@ protected function parseMarkdownPage(string $pageClass): AbstractMarkdownPage
return new $pageClass(
matter: $matter,
body: $body,
title: FindsTitleForDocument::get($this->slug, $matter, $body),
slug: $this->slug
);
}

protected function constructDynamicData(): void
{
$this->page->title = $this->findTitleForPage();

if ($this->page instanceof DocumentationPage) {
$this->page->category = $this->getDocumentationPageCategory();
}
}

protected function findTitleForPage(): string
{
if ($this->page instanceof BladePage) {
return Hyde::makeTitle($this->slug);
}

if ($this->page->matter('title')) {
return $this->page->matter('title');
}

return $this->findTitleFromMarkdownHeadings() ?? Hyde::makeTitle($this->slug);
}

protected function findTitleFromMarkdownHeadings(): ?string
{
foreach ($this->page->markdown()->toArray() as $line) {
if (str_starts_with($line, '# ')) {
return trim(substr($line, 2), ' ');
}
}

return null;
}

protected function getDocumentationPageCategory(): ?string
{
// If the documentation page is in a subdirectory,
// then we can use that as the category name.
// Otherwise, we look in the front matter.

return str_contains($this->slug, '/')
? Str::before($this->slug, '/')
: $this->page->matter('category');
}

public function get(): PageContract
{
return $this->page;
Expand Down
47 changes: 0 additions & 47 deletions packages/framework/src/Concerns/HasDynamicTitle.php
Original file line number Diff line number Diff line change
@@ -1,50 +1,3 @@
<?php

namespace Hyde\Framework\Concerns;

use Hyde\Framework\Hyde;

/**
* Find and get the title to use for a Markdown Document.
*
* First check the front matter for a title. If one is not found,
* it searches the Markdown for a level one heading. Falls back to
* generating a title from the slug if no other title could be found.
*
* @deprecated Move to action to generate when constructing a page.
*/
trait HasDynamicTitle
{
public function constructDynamicTitle(): void
{
if (! isset($this->title) || $this->title === '') {
$this->title = $this->findTitleForDocument();
}
}

public function findTitleForDocument(): string
{
if (isset($this->matter['title'])) {
return $this->matter['title'];
}

return $this->findTitleTagInMarkdown($this->body)
?: Hyde::makeTitle($this->slug);
}

/**
* Attempt to find the title based on the first H1 tag.
*/
protected function findTitleTagInMarkdown(string $stream): string|false
{
$lines = explode("\n", $stream);

foreach ($lines as $line) {
if (str_starts_with($line, '# ')) {
return trim(substr($line, 2), ' ');
}
}

return false;
}
}
9 changes: 2 additions & 7 deletions packages/framework/src/Contracts/AbstractMarkdownPage.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Hyde\Framework\Contracts;

use Hyde\Framework\Concerns\HasDynamicTitle;
use Hyde\Framework\Facades\Markdown;
use Hyde\Framework\Models\MarkdownDocument;

Expand All @@ -22,8 +21,6 @@
*/
abstract class AbstractMarkdownPage extends AbstractPage
{
use HasDynamicTitle;

public MarkdownDocument $markdown;

public array $matter;
Expand All @@ -33,16 +30,14 @@ abstract class AbstractMarkdownPage extends AbstractPage

public static string $fileExtension = '.md';

public function __construct(array $matter = [], string $body = '', string $title = '', string $slug = '', ?MarkdownDocument $markdownDocument = null)
public function __construct(array $matter = [], string $body = '', ?string $title = null, string $slug = '', ?MarkdownDocument $markdownDocument = null)
{
$this->matter = $matter;
$this->body = $body;
$this->title = $title;
$this->title = $title ?? $matter['title'] ?? '';
$this->slug = $slug;

$this->markdown = $markdownDocument ?? new MarkdownDocument($matter, $body);

$this->constructDynamicTitle();
}

public function markdown(): MarkdownDocument
Expand Down
11 changes: 0 additions & 11 deletions packages/framework/src/Models/Pages/DocumentationPage.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use Hyde\Framework\Contracts\AbstractMarkdownPage;
use Hyde\Framework\Contracts\RouteContract;
use Hyde\Framework\Models\Route;
use Illuminate\Support\Str;

class DocumentationPage extends AbstractMarkdownPage
{
Expand All @@ -24,16 +23,6 @@ class DocumentationPage extends AbstractMarkdownPage
public function __construct(array $matter = [], string $body = '', string $title = '', string $slug = '')
{
parent::__construct($matter, $body, $title, $slug);
$this->category = $this->getDocumentationPageCategory();
}

protected function getDocumentationPageCategory(): ?string
{
if (str_contains($this->slug, '/')) {
return Str::before($this->slug, '/');
}

return $this->matter['category'] ?? null;
}

/** @inheritDoc */
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/src/Services/RssFeedService.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function getXML(): string|false
protected function addItem(MarkdownPost $post): void
{
$item = $this->feed->channel->addChild('item');
$item->addChild('title', $post->findTitleForDocument());
$item->addChild('title', $post->title);
$item->addChild('link', $post->getCanonicalLink());
$item->addChild('guid', $post->getCanonicalLink());
$item->addChild('description', $post->getPostDescription());
Expand Down
8 changes: 1 addition & 7 deletions packages/framework/tests/Feature/AbstractPageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Hyde\Framework\Testing\Feature;

use Hyde\Framework\Concerns\HasDynamicTitle;
use Hyde\Framework\Contracts\AbstractMarkdownPage;
use Hyde\Framework\Contracts\AbstractPage;
use Hyde\Framework\Contracts\PageContract;
Expand Down Expand Up @@ -86,7 +85,7 @@ public function test_all_returns_collection_of_all_source_files_parsed_into_the_
{
Hyde::touch(('_pages/foo.md'));
$this->assertEquals(
collect([new MarkdownPage([], '', '', 'foo')]),
collect([new MarkdownPage([], '', 'Foo', 'foo')]),
MarkdownPage::all()
);
unlink(Hyde::path('_pages/foo.md'));
Expand Down Expand Up @@ -237,11 +236,6 @@ public function test_abstract_markdown_page_implements_page_contract()
$this->assertInstanceOf(PageContract::class, new class extends AbstractMarkdownPage {});
}

public function test_abstract_markdown_page_uses_has_dynamic_title_trait()
{
$this->assertContains(HasDynamicTitle::class, class_uses_recursive(AbstractMarkdownPage::class));
}

public function test_abstract_markdown_page_has_markdown_document_property()
{
$this->assertClassHasAttribute('markdown', AbstractMarkdownPage::class);
Expand Down
45 changes: 45 additions & 0 deletions packages/framework/tests/Feature/SourceFileParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Hyde\Framework\Testing\Feature;

use Hyde\Framework\Actions\SourceFileParser;
use Hyde\Framework\Hyde;
use Hyde\Framework\Models\Pages\BladePage;
use Hyde\Framework\Models\Pages\DocumentationPage;
use Hyde\Framework\Models\Pages\MarkdownPage;
Expand Down Expand Up @@ -59,4 +60,48 @@ public function test_documentation_page_parser()
$this->assertEquals('# Foo Bar', $page->body);
$this->assertEquals('Foo Bar Baz', $page->title);
}

public function test_dynamic_data_constructor_can_find_title_from_front_matter()
{
$this->markdown('_pages/foo.md', '# Foo Bar', ['title' => 'My Title']);
$page = MarkdownPage::parse('foo');
$this->assertEquals('My Title', $page->title);
}

public function test_dynamic_data_constructor_can_find_title_from_h1_tag()
{
$this->markdown('_pages/foo.md', '# Foo Bar');
$page = MarkdownPage::parse('foo');

$this->assertEquals('Foo Bar', $page->title);
}

public function test_dynamic_data_constructor_can_find_title_from_slug()
{
$this->markdown('_pages/foo-bar.md');
$page = MarkdownPage::parse('foo-bar');

$this->assertEquals('Foo Bar', $page->title);
}

public function test_documentation_page_parser_can_get_category_from_front_matter()
{
$this->markdown('_docs/foo.md', '# Foo Bar', ['category' => 'foo']);

$page = DocumentationPage::parse('foo');
$this->assertEquals('foo', $page->category);
}

public function test_documentation_page_parser_can_get_category_automatically_from_nested_page()
{
mkdir(Hyde::path('_docs/foo'));
touch(Hyde::path('_docs/foo/bar.md'));

/** @var DocumentationPage $page */
$page = DocumentationPage::parse('foo/bar');
$this->assertEquals('foo', $page->category);

unlink(Hyde::path('_docs/foo/bar.md'));
rmdir(Hyde::path('_docs/foo'));
}
}
27 changes: 0 additions & 27 deletions packages/framework/tests/Unit/DocumentationPageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Hyde\Framework\Testing\Unit;

use Hyde\Framework\Actions\SourceFileParser;
use Hyde\Framework\Hyde;
use Hyde\Framework\HydeServiceProvider;
use Hyde\Framework\Models\Pages\DocumentationPage;
Expand Down Expand Up @@ -129,30 +128,4 @@ public function test_compiled_pages_originating_in_subdirectories_get_output_to_
$page = (new DocumentationPage([], '', '', 'foo/bar'));
$this->assertEquals('docs/bar.html', $page->getOutputPath());
}

public function test_documentation_page_parser_can_get_category_from_front_matter()
{
$this->markdown('_docs/foo.md', '# Foo Bar', ['category' => 'foo']);

$parser = new SourceFileParser(DocumentationPage::class, 'foo');

/** @var DocumentationPage $page */
$page = $parser->get();
$this->assertEquals('foo', $page->category);
}

public function test_documentation_page_parser_can_get_category_automatically_from_nested_page()
{
mkdir(Hyde::path('_docs/foo'));
touch(Hyde::path('_docs/foo/bar.md'));

$parser = new SourceFileParser(DocumentationPage::class, 'foo/bar');

/** @var DocumentationPage $page */
$page = $parser->get();
$this->assertEquals('foo', $page->category);

unlink(Hyde::path('_docs/foo/bar.md'));
rmdir(Hyde::path('_docs/foo'));
}
}
Loading