Add rowspan support to DocBook reader. #10981
Conversation
tarleb
left a comment
There was a problem hiding this comment.
LGTM! I added some comments, but they are really just comments; none of them needs fixing.
src/Text/Pandoc/Readers/DocBook.hs
Outdated
| if n > 0 then Just n else Nothing | ||
| let numrows = maybe 0 maximum $ nonEmpty | ||
| $ map length bodyrows | ||
| let numrows = maybe 0 maximum $ nonEmpty |
There was a problem hiding this comment.
That trailing space is probably unintentional?
src/Text/Pandoc/Readers/DocBook.hs
Outdated
| import Data.List.NonEmpty (nonEmpty) | ||
| import Data.Maybe (catMaybes,fromMaybe,mapMaybe,maybeToList) | ||
| import Data.Text (Text) | ||
| import Data.Text (Text, unpack) |
There was a problem hiding this comment.
There are many modules that define an unpack function, so most modules in pandoc use the more explicit T.unpack. That's by no means a rule though, importing it this way is fine.
| let rowDistance mr = do | ||
| case readMaybe $ unpack mr :: Maybe Int of | ||
| Just moreRow -> RowSpan $ moreRow + 1 | ||
| _ -> 1 |
There was a problem hiding this comment.
Some linting tools will complain about this wildcard use, suggesting to either use a descriptive name like _notAnInt or to be explicit and match on Nothing. This can help to avoid confusion.
Doesn't need fixing, purely FYI.
There was a problem hiding this comment.
I was thinking about that, but figured it may be better to be consistent with how colspan was handled.
|
Thanks @tarleb! I've removed the trailing space and changed the unpack import. |
|
The test failure is unrelated to these changes, so I'm merging this. Thanks! |
This PR adds rowspan support to the DocBook reader by reading the
morerowsattribute in table cells, as mentioned in #10918. Previously, the rowspan was always set to 1 regardless of this attribute. I added a test based on the table in the issue as well.Besides that, I changed
parseTableto read all the rows intheadif there are multiple rows (previously, only the first row was read). It also considers the rows intheadwhen setting the number of columns. Previously, it only considered those in the body, so it failed to correctly parse tables if there were more cells in a row in the head than the body.