Closed lumi-a closed 2 days ago
Sorry for the very late response, but I just pushed some new changes, which affect some of your comments:
The redundant recursion has been removed
Shorthands will now be applied on non-sequence content. This is done via dynamically created show rules, though the implementation is very hacky and probably prone to errors.
As I couldn't think of any workaround for distinguishing strings from other content in math, shorthands are still applied on strings. However, in your example, it would be possible to include the comma in the string, so that they all form a single text node. This is of course still not ideal, but playing around with the text node grouping (e.g. by escaping some characters, as they are treated as their own nodes) is probably the best you can currently do.
Thank you for your response and for the improvements! I ran your tests and my above singleton-code against your new version, they all passed! Happy to close this issue :) 🌻
Background: I've been using your implementation of quick-maths to write a package handling Unicode super-/subscripts in maths-mode (thanks for the MIT license!), and have some questions about your implementation of quick-maths.
Possibly redundant recursion
Quick-maths applies
convert-sequence
to allsequence
s within amath.equation
via this code:This also works for sequences within sequences. However, in
convert-sequence
, you additionally applyconvert-sequence
to the children of the sequence itself as well, which seems redundant:I tested the package after replacing this line with
let children = seq.children
instead, and it still seems to work as expected. Please correct me if I'm wrong, but mappingconvert-sequence
over the children doesn't seem to change them anyway, because children seem to never besequence
s themselves. This would also explain why, in my tests of your current implementation, the run-time doesn't depend exponentially on the nesting-level of a sequence.Content that is not a sequence
The sequence type is declared as
#let sequence = $a b$.body.func()
, which seems to be an array of content. However, some users might want to replace single pieces of content, which is not possible, as those are stored without being wrapped in an array. For example:In the unexpected cases, the issue is that the content
[+]
is not part of a sequence. In the expected first example, the content[+]
is part of the sequence([a], [ ], [+], [ ], [b])
.I don't know how this could easily be solved, as we can't expect an upstream-change wrapping even single pieces of content into a sequence.
Perhaps, instead of running[Edit: This is not possible, as show-rules don't work on theconvert-sequence
on all sequences, it could be modified to run on all instances ofcontent
instead.content
-constructor]~~Or the code could be refactored to actually use recursion, by mapping over
seq.fields()
instead ofseq.children
. Unless you exclude children matching.func == math.equation
, the recursive solution has bad run-time for nested content: In the admittedly contrived example$a #[$b #[$c$]$]$
, the expressionb
would have the quick-maths-transformation applied to it twice, andc
three times. This is not the case forshow
-rules.~~ [Edit: This also is not possible: Mutation of content-fields is not allowed, and replacing a content-object by a mutated copy not feasible, because positional arguments of content-functions mustn't be passed as named arguments:]
Replacement of non-maths content
In the current implementation, all sequences within a
math.equation
are replaced. This includes sequences of non-maths-content, in particular strings. This seems unsolvable at the moment, because we seem unable to distinguish between strings and maths-content in math-mode:This currently is a small issue, because string-contents aren't broken apart like equation-contents are (
#$a+b "a+b"$.body.children
yields([a], [+], [b], [ ], [a+b])
), so they usually don't match positively against math-content, and so aren't replaced. As a consequence, the example-bug below is extremely contrived. Depending on the resolution of the "Content that is not a sequence" in the previous section, this bug might actually cause problems in regular use, though.Contrived example: Perhaps you type
1.5
a lot, but to be fancy, you would like it to be replaced by the fraction3/2
. Because$1.5$
is a single content-block and not a sequence, it currently can't be replaced by quick-maths, so you have quick-maths replace$1.5,$
instead. Now, you might use the number1.5
in a string as well, e.g. to refer to figure-numbers. Of course, we wouldn't want to have the text "Looking at figure 1.5, we see…" be replaced by "Looking at figure ³/₂ we see…". In this example, that's what happens, though:Note that the comma after
"1.5"
only had to be backslash-escaped because it's within the arguments-list of thecases
-function. In the even more contrived example$"From figure" "1.5", n=k$
, the bug is triggered as well. Note that the bug is not triggered for$"From figure 1.5", n=k$
because here1.5
is not an isolated string.