jeffreykegler / Marpa--R2

Parse any language you can describe in BNF
https://jeffreykegler.github.io/Marpa-web-site/
Other
155 stars 23 forks source link

In Marpa::R2::ASF, (full traverser) problem with return value from rh_value() #265

Closed ronsavage closed 7 years ago

ronsavage commented 8 years ago

In your code for the full traverser, the inner-most loop is:

          for my $new_value ( @{ $child_value } ) {
              push @new_results, [ @{$old_result}, $new_value ];
          }

I write that as:

    for my $new_value (@$child_value)
    {
        $$scratch{self} -> log(debug => "New value: <$new_value>");
       push @new_result, [@$old_result, $new_value];
   }

This was my first output:

shell> perl -Ilib scripts/parse.pl -max debug -d '11 Jan 1954'
New value: <11>
New value: <jan>
New value: <1954>
New value: <gregorian_year => [
1954
]

> New value: <gregorian_date => [
> 11,
> jan,
> gregorian_year=>[1954]
> ]
> 
> etc

Why on earth are there \n embedded in $new_value? If it's deliberate, OK, but I find them a PITA.

By turning the loop into:

    for my $new_value (@$child_value)
    {
        $new_value =~ s|\n||g;
    $$scratch{self} -> log(debug => "New value: <$new_value>");

    push @new_result, [@$old_result, $new_value];
}

I get:

New value: <11>
New value: <jan>
New value: <1954>
New value: <gregorian_year => [1954]>
New value: <gregorian_date => [11,jan,gregorian_year=>[1954]]>
ronsavage commented 8 years ago

I agree that this issue may /seem/ trivial, but when debugging with Data::Dumper::Concise, the \n are escaped, and I ended up with (ie for [:start], not for the above): Symbol name: [:start]. Result: [:start] => [ "date_string => [\n \"generic_date => [\n \"date_type => [\n \\"gregorian_date => [\\n 11,\\n \\\\"jan\\\\",\\n \\\\"gregorian_year => [\\\\n 1954\\\\n]\\\\n\\\\"\\n]\\n\\"\n]\n\"\n]\n[\n \"date_type => [\n \\"julian_date => [\\n 11,\\n \\\\"jan\\\\",\\n \\\\"year => [\\\\n 1954\\\\n]\\\\n\\\\"\\n]\\n\\"\n]\n\"\n]\n\"\n]\n" ] [ "date_string => [\n \"lds_ord_date => [\n \"date_value => [\n \\"generic_date => [\\n \\\\"date_type => [\\\\n \\\\\\\\"gregorian_date => [\\\\\\\\n 11,\\\\\\\\n \\\\\\\\\\\\\\\\"jan\\\\\\\\\\\\\\\\",\\\\\\\\n \\\\\\\\\\\\\\\\"gregorian_year => [\\\\\\\\\\\\\\\\n 1954\\\\\\\\\\\\\\\\n]\\\\\\\\\\\\\\\\n\\\\\\\\\\\\\\\\"\\\\\\\\n]\\\\\\\\n\\\\\\\\"\\\\n]\\\\n\\\\"\\n]\\n[\\n \\\\"date_type => [\\\\n \\\\\\\\"julian_date => [\\\\\\\\n 11,\\\\\\\\n \\\\\\\\\\\\\\\\"jan\\\\\\\\\\\\\\\\",\\\\\\\\n \\\\\\\\\\\\\\\\"year => [\\\\\\\\\\\\\\\\n 1954\\\\\\\\\\\\\\\\n]\\\\\\\\\\\\\\\\n\\\\\\\\\\\\\\\\"\\\\\\\\n]\\\\\\\\n\\\\\\\\"\\\\n]\\\\n\\\\"\\n]\\n\\"\n]\n\"\n]\n\"\n]\n" ]

jeffreykegler commented 8 years ago

full_traverser() is sample code, not part of the API -- which does not mean it should not be right. But it does mean that you can just make your own copy and hack to suit.

ronsavage commented 8 years ago

Understand! I have written my own version. But it's the arrayref returned by rh_value() which contains the \n.

rns commented 8 years ago

Hi Ron, is the code in repo somewhere? I'd take a look into that \n stuff if/when it is.

ronsavage commented 8 years ago

Hi rns. I've pushed my code to Genealogy-Gedcom-Date. Just unpack it and run: perl -Ilib scripts/parse.pl -max debug -d '11 Jan 1954' The date 'From 10 Jan 1954 to 11 Jan 1954' also works, but loses the 'from' and 'to'. Check line 568 of Date.pm to see where I clean it up (i.e. the output from the ASF). Note: The whole approach I copied from Jeffrey's sample of pushing onto @new_result will be wiped. It's 8:45 am here, so I'll start $work first :-(. I'm going to pass a Tree into the traverser and make each child a child of the 'current' node, so I can return the data without formatting. If it all works, it may become a stand-alone module for CPAN for users which to walk the ASF tree and get back a more 'normal' tree. PS I did not get an email re your comment, not the tiny blue dot at the top of this page which I get when Jeffrey comments. Don't know what's going on there. I am getting emails for his comments, too.

rns commented 8 years ago

@ronsavage: Thanks, I'll take a look.

BTW, I did get emails about both your comments,FWIW.

jeffreykegler commented 7 years ago

@ronsavage Your alternative code assumes child values are strings, and eliminates all newlines. While this is ideal for your application, it is easy to imagine it being disastrous for another. Each app has to adapt full_traverser() as needed.

In the above referenced commit, I've added some language to this effect in the doc. If that looks good to you, we can close this issue.

ronsavage commented 7 years ago

Hi Jeffrey

Yes, close it.

On 01/03/17 03:23, Jeffrey Kegler wrote:

Your alternative code assumes child values are strings, and eliminates all newlines. While this is ideal for your application, it is easy to imagine it being disastrous for another. Each app has to adapt |full_traverser()| as needed.

In the above referenced commit, I've added some language to this effect in the doc. If that looks good to you, we can close this issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffreykegler/Marpa--R2/issues/265#issuecomment-283087780, or mute the thread https://github.com/notifications/unsubscribe-auth/AABT-H06PZjPyi4hJqULpvUXbL_imXnzks5rhEn4gaJpZM4GSkhm.

-- Ron Savage - savage.net.au

jeffreykegler commented 7 years ago

Closed as fixed, as per the last 2 posts.