Skip to content

Commit dbc9e30

Browse files
Happypig375Happypig375
authored
Fix issue #213 (#249)
1 parent c8da0c5 commit dbc9e30

File tree

11 files changed

+120
-41
lines changed

11 files changed

+120
-41
lines changed

‎.github/workflows/Label.yml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,24 @@ jobs:
1111
with:
1212
fetch-depth: 0 # Fetch all commit history and tags, instead of the default fetch of only one commit
1313
ref: ${{ github.event.pull_request.head.sha }} # Checkout the PR branch
14-
- name: If there are changes in PublicApi.Shipped.txt, fail the workflow
14+
- name: If there are changes in PublicAPI.Shipped.txt, fail the workflow
1515
if: github.head_ref != 'action/ship-publicapi' # Same branch name specified in Release.yml
1616
run: |
17-
$changes = git diff --numstat --shortstat origin/${{ github.base_ref }}...HEAD -- '**/PublicApi.Shipped.txt' # Note that ** must expand to at least one folder here, this doesn't check root
17+
$changes = git diff --numstat --shortstat origin/${{ github.base_ref }}...HEAD -- '**/PublicAPI.Shipped.txt' # Note that ** must expand to at least one folder here, this doesn't check root
1818
Write-Output "$changes"
1919
if ($changes) {
20-
Write-Error "Changes detected in PublicApi.Shipped.txt files. Public API changes must be shipped through the release process, not in regular pull requests."
20+
Write-Error "Changes detected in PublicAPI.Shipped.txt files. Public API changes must be shipped through the release process, not in regular pull requests."
2121
exit 1
2222
}
2323
shell: pwsh
24-
- name: Label based on PublicApi.Unshipped.txt
24+
- name: Label based on PublicAPI.Unshipped.txt
2525
run: |
2626
if ("${{ github.head_ref }}" -eq "action/ship-publicapi") {
2727
$labels = @('Type/Housekeeping')
2828
Write-Output "This is a ship-publicapi PR, labeling as Type/Housekeeping"
2929
} else {
3030
# For regular PRs, check for API changes
31-
$changes = git diff origin/${{ github.base_ref }}...HEAD -- '**/PublicApi.Unshipped.txt'
31+
$changes = git diff origin/${{ github.base_ref }}...HEAD -- '**/PublicAPI.Unshipped.txt'
3232
Write-Output "$changes"
3333
3434
if ($changes) {

‎.github/workflows/Release-mark-shipped.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ function MarkShipped([string]$dir) {
3838
try {
3939
Push-Location $PSScriptRoot\..\..
4040

41-
foreach ($file in Get-ChildItem -re -in "PublicApi.Shipped.txt") {
41+
foreach ($file in Get-ChildItem -re -in "PublicAPI.Shipped.txt") {
4242
$dir = Split-Path -parent $file
4343
MarkShipped $dir
4444
}

‎CSharpMath.Core.Tests/Atom/MathListTest.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,8 @@ static void CheckListContents(MathList? list) {
173173
CheckAtomNucleusAndRange<Fraction>("", 13, 1),
174174
CheckAtomNucleusAndRange<LargeOperator>("∫", 14, 1),
175175
CheckAtomNucleusAndRange<Variable>("θ", 15, 1),
176-
// Comments are not given ranges as they won't affect typesetting
177-
CheckAtomNucleusAndRange<Comment>(":)", Range.UndefinedInt, Range.UndefinedInt),
178-
CheckAtomNucleusAndRange<Punctuation>(",", 16, 1)
176+
CheckAtomNucleusAndRange<Comment>(":)", 16, 1),
177+
CheckAtomNucleusAndRange<Punctuation>(",", 17, 1)
179178
);
180179
Assert.Collection(list.Atoms[2].Superscript,
181180
CheckAtomNucleusAndRange<Number>("13", 0, 2),

‎CSharpMath.Core.Tests/Display/TypesetterTests.cs‎

Lines changed: 92 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ internal static ListDisplay<TFont, TGlyph> ParseLaTeXToDisplay(string latex) =>
1616
private static readonly TFont _font = new TFont(20);
1717
private static readonly TypesettingContext<TFont, TGlyph> _context = BackEnd.TestTypesettingContext.Instance;
1818

19-
System.Action<IDisplay<TFont, TGlyph>?> TestList(int rangeMax, double ascent, double descent, double width, double x, double y,
19+
static System.Action<IDisplay<TFont, TGlyph>?> TestList((int, int) range, double ascent, double descent, double width, double x, double y,
2020
LinePosition linePos, int indexInParent, params System.Action<IDisplay<TFont, TGlyph>>[] inspectors) => d => {
2121
var list = Assert.IsType<ListDisplay<TFont, TGlyph>>(d);
2222
Assert.False(list.HasScript);
23-
Assert.Equal(new Range(0, rangeMax), list.Range);
23+
Assert.Equal(new Range(range.Item1, range.Item2), list.Range);
2424
Approximately.Equal(ascent, list.Ascent);
2525
Approximately.Equal(descent, list.Descent);
2626
Approximately.Equal(width, list.Width);
@@ -29,10 +29,17 @@ internal static ListDisplay<TFont, TGlyph> ParseLaTeXToDisplay(string latex) =>
2929
Assert.Equal(indexInParent, list.IndexInParent);
3030
Assert.Collection(list.Displays, inspectors);
3131
};
32-
void TestOuter(string latex, int rangeMax, double ascent, double descent, double width,
32+
static System.Action<IDisplay<TFont, TGlyph>?> TestList(int rangeMax, double ascent, double descent, double width, double x, double y,
33+
LinePosition linePos, int indexInParent, params System.Action<IDisplay<TFont, TGlyph>>[] inspectors) =>
34+
TestList((0, rangeMax), ascent, descent, width, x, y, linePos, indexInParent, inspectors);
35+
static void TestOuter(string latex, int rangeMax, double ascent, double descent, double width,
3336
params System.Action<IDisplay<TFont, TGlyph>>[] inspectors) =>
3437
TestList(rangeMax, ascent, descent, width, 0, 0, LinePosition.Regular, Range.UndefinedInt, inspectors)
3538
(ParseLaTeXToDisplay(latex));
39+
static void TestOuter(string latex, (int, int) range, double ascent, double descent, double width,
40+
params System.Action<IDisplay<TFont, TGlyph>>[] inspectors) =>
41+
TestList(range, ascent, descent, width, 0, 0, LinePosition.Regular, Range.UndefinedInt, inspectors)
42+
(ParseLaTeXToDisplay(latex));
3643

3744
/// <summary>Makes sure that a single codepoint of various atom types have the same measured size.</summary>
3845
[Theory, InlineData("x"), InlineData("2"), InlineData(","), InlineData("+"), InlineData("Σ"), InlineData("𝑥")]
@@ -67,18 +74,18 @@ public void TestVariablesNumbersAndOrdinaries(string latex) =>
6774
Assert.Equal(40, line.Width);
6875
});
6976
[Theory]
70-
[InlineData("%\n1234", "1234")]
71-
[InlineData("12.b% comment ", "12.𝑏")]
72-
[InlineData("|`% \\notacommand \u2028@/", "|`@/")]
73-
public void TestIgnoreComments(string latex, string text) =>
74-
TestOuter(latex, 4, 14, 4, 40,
77+
[InlineData("%\n1234", "1234", 1, 4)]
78+
[InlineData("12.b% comment ", "12.𝑏", 0, 4)]
79+
[InlineData("|`% \\notacommand \u2028@/", "|`@/", 0, 5)]
80+
public void TestIgnoreComments(string latex, string text, int rangeStart, int rangeEnd) =>
81+
TestOuter(latex, (rangeStart, rangeEnd), 14, 4, 40,
7582
d => {
7683
var line = Assert.IsType<TextLineDisplay<TFont, TGlyph>>(d);
7784
Assert.Equal(4, line.Atoms.Count);
7885
Assert.All(line.Atoms, Assert.IsNotType<Atom.Atoms.Comment>);
7986
Assert.Equal(text, string.Concat(line.Text));
8087
Assert.Equal(new PointF(), line.Position);
81-
Assert.Equal(new Range(0, 4), line.Range);
88+
Assert.Equal(new Range(rangeStart, rangeEnd), line.Range);
8289
Assert.False(line.HasScript);
8390

8491
Assert.Equal(14, line.Ascent);
@@ -533,5 +540,81 @@ public void TestColor() =>
533540
Assert.Null(line.TextColor);
534541
}
535542
);
543+
[Fact]
544+
public void Issue213() {
545+
float charWidth = 10;
546+
float mathUnit = 20f / 18f;
547+
foreach (var space in new[] { "", "\\," })
548+
// 5 mu spacing between = (Relation) and - (Unary Operator, aka Ordinary)
549+
TestOuter($"{space}=-1", (space.Length / 2, 3), 14, 4, 3 * charWidth + 5 * mathUnit, // expected: - becomes UnaryOperator then typesetted as Ordinary
550+
d => {
551+
var textBefore = Assert.IsType<TextLineDisplay<TFont, TGlyph>>(d);
552+
Assert.Equal(3 * charWidth + 5 * mathUnit, textBefore.Width);
553+
Assert.Equal("=\u22121", string.Concat(textBefore.Text));
554+
Assert.Equal(new Range(space.Length / 2, 3), textBefore.Range);
555+
Assert.Collection(textBefore.Atoms,
556+
a => Assert.Equal("=", Assert.IsType<Atom.Atoms.Relation>(a).Nucleus),
557+
a => Assert.Equal("\u2212", Assert.IsType<Atom.Atoms.Ordinary>(a).Nucleus),
558+
a => Assert.Equal("1", Assert.IsType<Atom.Atoms.Ordinary>(a).Nucleus));
559+
});
560+
foreach (var (nonDisplayedAtomThatCreatesNewDisplayLine, additionalWidth) in new[] { (@"\, ", 3 * mathUnit), (@"\displaystyle ", 0) })
561+
TestOuter($@"={nonDisplayedAtomThatCreatesNewDisplayLine}-1", 4, 14, 4, 3 * charWidth + 5 * mathUnit + additionalWidth, // issue 213: inserting a space between them should still work
562+
eq => {
563+
var textBefore = Assert.IsType<TextLineDisplay<TFont, TGlyph>>(eq);
564+
Assert.Equal(new PointF(), textBefore.Position);
565+
Assert.Equal(charWidth, textBefore.Width);
566+
Assert.Equal("=", string.Concat(textBefore.Text));
567+
Assert.Equal(new Range(0, 1), textBefore.Range);
568+
Assert.Equal("=", Assert.IsType<Atom.Atoms.Relation>(Assert.Single(textBefore.Atoms)).Nucleus);
569+
}, m1 => {
570+
var textAfter = Assert.IsType<TextLineDisplay<TFont, TGlyph>>(m1);
571+
Assert.Equal(new PointF(10 + 5 * mathUnit + additionalWidth, 0), textAfter.Position);
572+
Assert.Equal(2 * charWidth, textAfter.Width);
573+
Assert.Equal("\u22121", string.Concat(textAfter.Text));
574+
Assert.Equal(new Range(2, 2), textAfter.Range);
575+
Assert.Collection(textAfter.Atoms,
576+
a => Assert.Equal("\u2212", Assert.IsType<Atom.Atoms.Ordinary>(a).Nucleus),
577+
a => Assert.Equal("1", Assert.IsType<Atom.Atoms.Ordinary>(a).Nucleus));
578+
});
579+
TestOuter("=%comment\n-1", 4, 14, 4, 3 * charWidth + 5 * mathUnit, // same should apply to Comment atoms
580+
d => {
581+
var textBefore = Assert.IsType<TextLineDisplay<TFont, TGlyph>>(d);
582+
Assert.Equal(3 * charWidth + 5 * mathUnit, textBefore.Width);
583+
Assert.Equal("=\u22121", string.Concat(textBefore.Text));
584+
Assert.Equal(new Range(0, 4), textBefore.Range);
585+
Assert.Collection(textBefore.Atoms,
586+
a => Assert.Equal("=", Assert.IsType<Atom.Atoms.Relation>(a).Nucleus),
587+
a => Assert.Equal("\u2212", Assert.IsType<Atom.Atoms.Ordinary>(a).Nucleus),
588+
a => Assert.Equal("1", Assert.IsType<Atom.Atoms.Ordinary>(a).Nucleus));
589+
});
590+
}
591+
[Fact]
592+
public void SpacingBetweenNumbers() {
593+
// Numbers that have non-display atoms between them should not be fused together.
594+
foreach (var (nonDisplayedAtomThatCreatesNewDisplayLine, additionalWidth) in new[] { (@"\quad ", 20), (@"\displaystyle ", 0) })
595+
TestOuter($"2{nonDisplayedAtomThatCreatesNewDisplayLine}3", 3, 14, 4, 10 + additionalWidth + 10,
596+
d => {
597+
var line = Assert.IsType<TextLineDisplay<TFont, TGlyph>>(d);
598+
Assert.Equal(new PointF(), line.Position);
599+
Assert.Equal("2", Assert.IsType<Atom.Atoms.Ordinary>(Assert.Single(line.Atoms)).Nucleus);
600+
Assert.Equal("2", string.Concat(line.Text));
601+
Assert.Equal(new Range(0, 1), line.Range);
602+
Assert.False(line.HasScript);
603+
Assert.Equal(14, line.Ascent);
604+
Assert.Equal(4, line.Descent);
605+
Assert.Equal(10, line.Width);
606+
},
607+
d => {
608+
var line = Assert.IsType<TextLineDisplay<TFont, TGlyph>>(d);
609+
Assert.Equal(new PointF(10 + additionalWidth, 0), line.Position);
610+
Assert.Equal("3", Assert.IsType<Atom.Atoms.Ordinary>(Assert.Single(line.Atoms)).Nucleus);
611+
Assert.Equal("3", string.Concat(line.Text));
612+
Assert.Equal(new Range(2, 1), line.Range);
613+
Assert.False(line.HasScript);
614+
Assert.Equal(14, line.Ascent);
615+
Assert.Equal(4, line.Descent);
616+
Assert.Equal(10, line.Width);
617+
});
618+
}
536619
}
537620
}

‎CSharpMath.Rendering.Tests/MathDisplay/Abs.png‎

157 Bytes
Loading

‎CSharpMath.Rendering.Tests/MathInline/Abs.png‎

157 Bytes
Loading

‎CSharpMath/Atom/MathAtom.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,7 @@ public void Fuse(MathAtom otherAtom) {
7272
FusedAtoms.Add(otherAtom);
7373
}
7474
Nucleus += otherAtom.Nucleus;
75-
IndexRange = new Range(IndexRange.Location,
76-
IndexRange.Length + otherAtom.IndexRange.Length);
75+
IndexRange += otherAtom.IndexRange;
7776
Subscript = otherAtom.Subscript;
7877
Superscript = otherAtom.Superscript;
7978
}

‎CSharpMath/Atom/MathList.cs‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,32 +53,28 @@ public MathList Clone(bool finalize) {
5353
foreach (var atom in Atoms)
5454
newList.Add(atom.Clone(finalize));
5555
} else {
56+
MathAtom? prevNode = null;
57+
int prevDisplayedIndex = -1;
5658
foreach (var atom in Atoms) {
57-
if (atom is Comment) {
58-
var newComment = atom.Clone(finalize);
59-
newComment.IndexRange = Range.NotFound;
60-
newList.Add(newComment);
61-
continue;
62-
}
63-
var prevNode = newList.Last;
6459
var newNode = atom.Clone(finalize);
65-
if (atom.IndexRange == Range.Zero) {
66-
int prevIndex =
67-
prevNode?.IndexRange.Location + prevNode?.IndexRange.Length ?? 0;
68-
newNode.IndexRange = new Range(prevIndex, 1);
69-
}
70-
switch (prevNode, newNode) {
60+
if (newNode.IndexRange == Range.Zero)
61+
newNode.IndexRange = new Range(prevNode is { } prev ? prev.IndexRange.Location + prev.IndexRange.Length : 0, 1);
62+
switch (prevDisplayedIndex == -1 ? null : newList[prevDisplayedIndex], newNode) {
63+
// NOTE: The left pattern does not include UnaryOperator. Just try "1+++2" and "1++++2" in any LaTeX rendering engine.
7164
case (null or BinaryOperator or Relation or Open or Punctuation or LargeOperator, BinaryOperator b):
7265
newNode = b.ToUnaryOperator();
7366
break;
7467
case (BinaryOperator b, Relation or Punctuation or Close):
75-
newList.Last = b.ToUnaryOperator();
68+
newList[prevDisplayedIndex] = b.ToUnaryOperator();
7669
break;
77-
case (Number n, Number _) when n.Superscript.IsEmpty() && n.Subscript.IsEmpty():
78-
n.Fuse(newNode);
79-
continue; // do not add the new node; we fused it instead.
8070
}
71+
if ((prevNode, newNode) is (Number { Superscript.Count: 0, Subscript.Count: 0 } n, Number)) {
72+
n.Fuse(newNode);
73+
continue; // do not add the new node; we fused it instead.
74+
}
75+
if (newNode is not (Comment or Space or Style)) prevDisplayedIndex = newList.Count; // Corresponds to atom types that use continue; in Typesetter.CreateLine
8176
newList.Add(newNode);
77+
prevNode = newNode;
8278
}
8379
}
8480
return newList;

‎CSharpMath/Atom/Range.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace CSharpMath.Atom {
1010
/// <summary>
1111
/// Corresponds to a range of <see cref="MathAtom"/>s before finalization.
1212
/// This value is tracked in finalized <see cref="MathAtom"/>s and <see cref="Display.IDisplay{TFont, TGlyph}"/>s,
13-
/// for utilization in CSharpMath.Editor to construct MathListIndexes from <see cref="Display.IDisplay{TFont, TGlyph}"/>s.
13+
/// for use in <see cref="Editor.Extensions.IndexForPoint{TFont, TGlyph}(Display.IDisplay{TFont, TGlyph}, Display.FrontEnd.TypesettingContext{TFont, TGlyph}, System.Drawing.PointF)"/>.
1414
/// </summary>
1515
public readonly struct Range : IEquatable<Range> {
1616
public const int UndefinedInt = int.MinValue;

‎CSharpMath/Display/InterElementSpaces.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ static int GetInterElementSpaceArrayIndexForType(MathAtom atomType, bool row) =>
5252
var multiplier =
5353
Spaces[leftIndex, rightIndex] switch {
5454
Invalid => throw new InvalidCodePathException
55-
($"Invalid space between {left.TypeName} and {right.TypeName}"),
55+
($"Invalid space between {left.TypeName} and {right.TypeName}. The {nameof(Atoms.BinaryOperator)} should have been converted to a {nameof(Atoms.UnaryOperator)} during {nameof(MathList)} {nameof(MathList.Clone)}, then converted to an {nameof(Atoms.Ordinary)} during {nameof(Typesetter)} preparation."),
5656
None => 0,
5757
Thin => 3,
5858
NsThin => style < LineStyle.Script ? 3 : 0,

0 commit comments

Comments
 (0)