prune XMDuals with mirrored single tokens #1327
Conversation
t/complex/physics.xml
Outdated
| <XMTok meaning="gradient"/> | ||
| <XMTok name="nabla" role="OPERATOR">∇</XMTok> | ||
| </XMDual> | ||
| <XMTok font="italic" meaning="gradient" name="nabla" role="OPERATOR">∇</XMTok> |
There was a problem hiding this comment.
I noticed that font="italic" gets added to the tokens when merged, I was wondering what causes that and how to avoid it - since I am pretty sure we don't want it added for the cases in the tests.
There was a problem hiding this comment.
Ok, figured it out, simply do not overwrite the presentation XMTok's attributes with attributes of the parent XMDual, in the cases where both have the same attribute. With a special exception where xml:id gets overwritten to be the XMDual id, in case it was being referenced from somewhere else.
… XMDual attrs, unless xml:id
|
Should be good for a review. |
lib/LaTeXML/Core/Document.pm
Outdated
| $self->replaceNode($dual, $presentation); | ||
| # transfer the attributes after replacing, so that the bookkeeping has been undone | ||
| for my $key (keys %transfer_attrs) { | ||
| if (($key eq 'xml:id') || !$presentation->getAttribute($key)) { |
There was a problem hiding this comment.
Some of the dual's attributes should override the presentation's attributes, certainly role, meaning, probably anything from @CONTENT_TRANSFER_ATTRS.
Also, theoretically if more than 1 of (dual,presentation,content)have ids, all referrers to the extras should get updated to use the one you keep.
There was a problem hiding this comment.
Thanks, hadn't thought that part through to be honest, and the changes to the tests are Good - e.g. \Langle from mathbbol and \Ydown from stmaryrd now once again have the correct roles.
There was a problem hiding this comment.
Ready for re-review
…et consistent overrides, concatenation, etc
|
This PR is now magically updated with the improved refactoring by @brucemiller , as you can see in the commit history 👍 |
|
Got no more excuses... |
Continues the work of #1309 to now also compress duals with single mirrored tokens.
The changes to the test suite are a very good illustration of the exact effect here.
I stumbled on this while working on the a11y PR #1305 , and wanted to detach it into a standalone PR, to make merging the various pieces easier.