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

Fix missing top or right border in LaTeX output #337

Merged
merged 2 commits into from
Jul 15, 2021
Merged

Fix missing top or right border in LaTeX output #337

merged 2 commits into from
Jul 15, 2021

Conversation

huaixv
Copy link
Contributor

@huaixv huaixv commented Jul 14, 2021

Hi David, thank you for creating this great package.

I recently use flextable in bookdown to format and display some complex tables.
And in some cases the LaTeX output is missing some table borders,
either when a merged cell occurs on the rightmost side of a table, or when the header is deleted.

I think issue #329 and #176 also report similar problems.

After digging into the source code, I found that there may be something wrong with the augment_(top_)borders function.

For example, when augment_borders function computes if a right border should be emitted,
it checks whether the col_id matches the col_id of the rightmost column,
this is ok for ordinary cells, but not the case for merged ones.

Instead, we can take rowspan into consideration and check if numeric_col_id + rowspan == no._of_columns + 1.

Also, when this function emits top border (hhline),
it checks if ft_row_id is 1 and if the part is the first of all three table parts (which is always header).
So when the table header is deleted, no top border will be produced.

And we can change its behavior to check if part is the first of existing table parts (it's body in case header is missing).

As to the coding style, I'm new to R lang, and I think there may be better ways to express these checks.
Please feel free to correct the code if you have better ideas. :-)

@davidgohel
Copy link
Owner

Thanks, this is really welcome.

I can't review that for now. I will try next week, but I don't have much time available, sorry if it takes times!

@davidgohel davidgohel self-assigned this Jul 14, 2021
@davidgohel
Copy link
Owner

Can't wait... Thanks for this fix.

@davidgohel davidgohel merged commit 037ed42 into davidgohel:master Jul 15, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants