Feature/tag texts #1345
Feature/tag texts #1345
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1345 +/- ##
===========================================
- Coverage 86.72% 86.71% -0.01%
===========================================
Files 337 337
Lines 84145 84147 +2
Branches 4769 4771 +2
===========================================
- Hits 72971 72968 -3
- Misses 11151 11156 +5
Partials 23 23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I would suggest to always consider tags to be a triple of strings [left, tag, right]. If one does not need the fences, once can leave them empty. This would avoid a all the case analysis for Array and would also simplify types.
Is there a reason why we would not want that?
| * @param {string} tag The tag name (e.g., 1). | ||
| * @param {string} tagId The unique id for that tag (e.g., mjx-eqn:1). | ||
| * @param {string} tagFormat The formatted tag (e.g., "(1)"). | ||
| * @param {string|string[]} tagFormat The formatted tag (e.g., "$\bullet$" or ['(','1',')']). |
There was a problem hiding this comment.
Would it not be easier to always have this as [string, string, string] return type and then use empty strings if no fence is used. That would allow for (left) tags like 1) to be written as ['', '1', ')'].
There was a problem hiding this comment.
That would allow for (left) tags like 1) to be written as
['', '1', ')']
That is allowed already, or (as you are suggesting filtering out the blank entries) it could be ['1', ')']. See my comment below.
| return format | ||
| ? `${left}${format}{${tag}}${right}` | ||
| : `${left}${tag}${right}`; | ||
| return format ? [left, `${format}{${tag}}`, right] : [left, tag, right]; |
There was a problem hiding this comment.
How about:
| return format ? [left, `${format}{${tag}}`, right] : [left, tag, right]; | |
| return [left, `format ? ${format}{${tag}} : tag`, right]; |
There was a problem hiding this comment.
The back-tics are in the wrong place, but otherwise a good idea.
| 'node', | ||
| 'mrow', | ||
| ParseUtil.internalMath(parser, tag), | ||
| ParseUtil.internalMath(parser, Array.isArray(tag) ? tag.join('') : tag), |
There was a problem hiding this comment.
This could then be simplified to just tag.join('').
| : format.match(/^(\(|\[|\{)(.*)(\}|\]|\))$/)?.slice(1) || [format]; | ||
| const mml = new TexParser( | ||
| '\\text{' + this.currentTag.tagFormat + '}', | ||
| tag.map((part) => `\\text{${part}}`).join(''), |
There was a problem hiding this comment.
This would need to filter out the empty strings.
There was a problem hiding this comment.
If SRE is going to use the three separate pieces to isolate the tag from the left- and right-hand parts, then don't we need to keep blank ones as empty mtext elements so that the middle one is always the tag. Otherwise how you could SRE tell whether a two-mtext pair was left-plus-tag or tag-plus-right?
Here we either get a single mtext (no question about what is the tag) or a triple of mtext nodes (middle is tag), so no ambiguity.
I could turn the [format] into ['', format, ''] so it is always three. But this is only what will happen in TeX input. I'm wondering if there shouldn't be something in the MathML input to split up a tag like is being done here, as well.
| const format = this.currentTag.tagFormat; | ||
| const tag = Array.isArray(format) | ||
| ? format | ||
| : format.match(/^(\(|\[|\{)(.*)(\}|\]|\))$/)?.slice(1) || [format]; |
There was a problem hiding this comment.
This could be simplified if we always assume a triple.
That would be a potential breaking change, which we aren't supposed to make in minor versions. So this is for backward compatibility, mainly with the I suppose the breaking up of the string could be moved to the |
2 participants
This PR changes the handling of tags to make three separate
mtextnodes: one for the left delimiter, one for the tag number, and one for the right delimiter.The
formatTag()andformatRef()functions now take either a string or a triple of strings to use for the tag format. If it is just a string, then parentheses, brackets, and braces are removed from the beginning and ending of the thing to make the triple, otherwise, the string is used as is (and only a singlemtextwill be created).The tests are updated to take these changes into account.