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

Implement yaml metadata hook #3

Merged
merged 7 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
Revision history for pmarkdown and the Markdown::Perl module.

1.08 - ?

- Add a new yaml_metadata hook to receive the parsed YAML.

1.07 - 2024-05-25

- Allow to specify a link content through the resolve_link_ref hook.
Expand Down
9 changes: 8 additions & 1 deletion lib/Markdown/Perl.pm
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ sub set_mode {
return;
}

Readonly::Array my @VALID_HOOKS => qw(resolve_link_ref);
Readonly::Array my @VALID_HOOKS => qw(resolve_link_ref yaml_metadata);

sub set_hooks {
my ($this, %hooks) = &_get_this_and_args; ## no critic (ProhibitAmpersandSigils)
Expand Down Expand Up @@ -324,6 +324,13 @@ optionally a C<title> key containing the title of the key. The hash-ref can also
contain a C<content> key, in which case its value should be a span of HTML which
will replace whatever would have been used for the link content.

=item *

C<yaml_metadata>: this hook will trigger if there is valid (!) YAML metadata in
the file and will give you a hashref as an argument. If the hook throws a die(),
the Markdown parsing will stop as the die() needs to be handled by your code.
This allows for conditional parsing based on values in the metadata section.

=back

=head1 AUTHOR
Expand Down
10 changes: 6 additions & 4 deletions lib/Markdown/Perl/BlockParser.pm
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ sub process {
# Done at a later stage, as escaped characters don’t have their Markdown
# meaning, we need a way to represent that.

# Note: for now, nothing is done with the extracted metadata.
$this->_parse_yaml_metadata() if $this->get_parse_file_metadata eq 'yaml';

while (defined (my $l = $this->next_line())) {
Expand Down Expand Up @@ -372,11 +371,14 @@ sub _parse_yaml_metadata {
my $metadata = eval { YAML::Tiny->read_string($+{YAML}) };
if ($EVAL_ERROR) {
pos($this->{md}) = 0;
adriaandens marked this conversation as resolved.
Show resolved Hide resolved
return;
carp 'YAML Metadata (Markdown frontmatter) is invalid.';
return 1;
Copy link
Owner

Choose a reason for hiding this comment

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

You can revert this line and line 381 to just return.

}
if(exists($this->{pmarkdown}{hooks}{yaml_metadata})) {
$this->{pmarkdown}{hooks}{yaml_metadata}->($metadata->[0]);
}
}

return;
return 1;
}

# https://spec.commonmark.org/0.30/#atx-headings
Expand Down
5 changes: 2 additions & 3 deletions lib/Markdown/Perl/Options.pm
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,8 @@ sub _word_list {
=head3 B<parse_file_metadata> I<(enum, default: yaml)>

This option controls whether the parser accepts optional metadata at the
beginning of the file. Currently, it does nothing with these metadata, even when
they are accepted. In the future they might be used to build complete HTML file
instead of just fragment.
beginning of the file. The module does nothing with the metadata itself but you
can configure a hook to intercept the YAML.

The possible values are:

Expand Down
71 changes: 71 additions & 0 deletions t/501-hooks-yaml-metadata.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
use strict;
use warnings;
use utf8;

use Markdown::Perl 'convert', 'set_hooks';
use Test::More;
use Test2::Tools::Warnings;
use Test2::Tools::Exception;

my $p = Markdown::Perl->new();
my $page = <<EOF;
---
name: Mark is down
draft: false
number: 42
---
# Mark is down!

I repeat: "Mark is down!"
EOF

my $invalid_page = <<EOF;
---
name: Mark is down
draft: false
number: 42
---
# Mark is down!

I repeat: "Mark is down!"
EOF

# Test 1: Check if we can get a string value
{
sub hook_is_name_mark {
my $x = shift;
ok(exists($x->{name}) && $x->{name} eq 'Mark is down', "key 'name' was retrieved and validated as being 'Mark is down'");
}
$p->set_hooks(yaml_metadata => \&hook_is_name_mark);
$p->convert($page);
}

# Test 2: Validate that hook is not called if yaml is invalid
Copy link
Owner

Choose a reason for hiding this comment

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

Are you missing this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, i forgot to git add the test file.
I also tried adding tests for the carp behaviour with Test::Carp but somehow that causes a different codepath to be taken and does not give the expected result whatever I try. :(

For example below, for unknown reasons to me, it does not call the hook (but if i run it as $p->convert($page) it does).

# Test 4: What happens if inside the hook we die()
use Test::Carp;

{
  sub hook_die {
    print "Dying ongoing...\n";
    die "We just die\n";
  }
  $p->set_hooks(yaml_metadata => \&hook_die);
  does_carp_that_matches($p->can("convert"), $page, qr/Hook.+crash/);
}

Copy link
Owner

Choose a reason for hiding this comment

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

I expect that this is because Test::Carp is incompatible with Test2 (so the hook is called but not caught by the test util). Instead you can use Test2::Tools::Warnings (which I believe is already included automatically by Test2::V0.

Also it might be because I expect that the current code dies in the call to convert because it’s dereferencing an undef $blocks returned by _parse (this will problem will go away once the exception is just propagated to the caller).

{
my $hook_called = 0;
sub hook_called {
$hook_called = 1;
}
$p->set_hooks(yaml_metadata => \&hook_called);
ok(!$hook_called, "Hook was not called because metadata was invalid.");
$p->convert($invalid_page);
}

# Test 3: Validate that invalid yaml causes a carp()
{
sub hook {
}
$p->set_hooks(yaml_metadata => \&hook);
like(warning { $p->convert($invalid_page) }, qr/invalid/, "Got expected warning");
}

# Test 4: What happens if inside the hook we die()
{
sub hook_die {
die "last words";
}
$p->set_hooks(yaml_metadata => \&hook_die);
like( dies { $p->convert($page) }, qr/last words/, "The hook correctly died.");
}

done_testing;