Mappers and table

[hannesm]

Dear Daniel,

thanks a lot for your recent release 0.3. I encountered some strange behaviour when using Mapper and tables:

My input data is:

# let data = {__||a||b|
  |-|-|-|
  |a|b|c|
  |a| | |
  | |b| ||__};;

Now, since 0.3, the standard html output looks great:

# print_endline @@ Cmarkit_html.of_doc ~safe:true (Cmarkit.Doc.of_string ~strict:false data);;
<div role="region"><table>
<tr>
<th>a</th>
<th></th>
<th>b</th>
</tr>
<tr>
<td>a</td>
<td>b</td>
<td>c</td>
</tr>
<tr>
<td>a</td>
<td></td>
<td></td>
</tr>
<tr>
<td></td>
<td>b</td>
<td></td>
</tr>
</table></div>

Now, I would like to use a Mapper -- as a first thing I just use the identity:

# let mapper = Cmarkit.Mapper.(make ~block:(fun _ _ -> default) ());;
# print_endline @@ Cmarkit_html.of_doc ~safe:true (Cmarkit.Mapper.map_doc mapper (Cmarkit.Doc.of_string ~strict:false data));;
<div role="region"><table>
<tr>
<th>a</th>
<th>b</th>
<th></th>
</tr>
<tr>
<td>a</td>
<td>b</td>
<td>c</td>
</tr>
<tr>
<td>a</td>
<td></td>
<td></td>
</tr>
<tr>
<td>b</td>
<td></td>
<td></td>
</tr>
</table></div>

Here, the heading moved b into the second column, and b in the last row moved to the first column. That's surprising for me.

If I use a slightly different mapper (which should be the identity as well), it all works again:

# let mapper2 = Cmarkit.Mapper.(make ~block:(fun _ -> function Cmarkit.Block.Blocks _ -> default | x -> ret x) ());;
# print_endline @@ Cmarkit_html.of_doc ~safe:true (Cmarkit.Mapper.map_doc mapper2 (Cmarkit.Doc.of_string ~strict:false data));;
<div role="region"><table>
<tr>
<th>a</th>
<th></th>
<th>b</th>
</tr>
<tr>
<td>a</td>
<td>b</td>
<td>c</td>
</tr>
<tr>
<td>a</td>
<td></td>
<td></td>
</tr>
<tr>
<td></td>
<td>b</td>
<td></td>
</tr>
</table></div>

This works again nicely -- so I don't quite understand whether the behaviour is expected or not?

[dbuenzli]

This code is wrong it should not List.filter_map, it should List.map and if the inline mapper returns None it should substitute an Inline.empty.

Basically now it suppresses empty cells so that shifts those that are non-empty at the beginning.

Your mapper2 is not equivalent to the first one: you are defaulting only the Blocks, case. All for all other cases you simply return the value without threading the mapper in the value.

[hannesm]

Thanks for your insight and pointing to the code that should be revised. The mapper2 indeed was a first shot at figuring out where the issue resides (and then I got demotivated and posted an issue instead of further investigating the root). For me and my restricted use of cmarkit (plus my sample documents), mapper2 works nicely -- but of course I'll revise my code once this issue is fixed and released.

Thanks for your time.