-
Notifications
You must be signed in to change notification settings - Fork 391
floatref - avoid caption duplication on Div Crossref with captioned Markdown table #13055
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
Conversation
… the table this is because Quarto floatref processes the table caption and it needs to be removed from the table element so that it doesn't get inserted twice
including the internal one from computation when using div attribute
… with markdown captioned tables
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
| -- table caption should be removed from the table as we'll handle it | ||
| table.caption.long = pandoc.Blocks({}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pandoc 3.6.1 added a pandoc.Caption constructor, so the cleanest variant would probably be
table.caption = pandoc.Caption{}This would also make sure that the short caption is reset. The short caption is intended to be used in the List of Tables, but it's not set in the default Markdown reader. But if that ever changes, then we capture that case as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I missed that. Changed done.
The short caption is intended to be used in the List of Tables
I wonder how we support that in Quarto. This would interesting support.
|
This is a good improvement, but I don't think it's a bug, and so we don't need the hotfix. My understanding is that the Div syntax has always required a content element and a caption element. If it worked before it was an accident; we never promised it would work. |
I assumed from this code in 1.6 that we were supposed to support it quarto-cli/src/resources/filters/quarto-pre/parsefiguredivs.lua Lines 237 to 269 in 5fc64f6
As you can see, the logic is
So it was not document in our website, but it was working from this code base. We had a caption removal at
which was removed accidentaly in 8bec5e3 So, based on Codebase this was working. I am fine to not do the backport, but it seemed a good candidate based on this history. |
closes #13051
when caption is taken on the table element, remove it from the table
this is because Quarto floatref processes the table caption and it needs to be removed from the table element so that it doesn't get inserted twice
This happens with syntax like
This PR adds also
Note
The fix is a candidate for a backport as it was a regression in 1.7 from 8bec5e3
I'll do a PR once validated