Skip to content

Commit

Permalink
Review fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
ygra committed Aug 15, 2020
1 parent 4717e58 commit b6582a3
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 61 deletions.
7 changes: 3 additions & 4 deletions src/WpfMath/Atoms/BigOperatorAtom.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ protected override Box CreateBoxCore(TexEnvironment environment)
// Make all component boxes equally wide.
var maxWidth = Math.Max(Math.Max(baseBox.Width, upperLimitBox == null ? 0 : upperLimitBox.Width),
lowerLimitBox == null ? 0 : lowerLimitBox.Width);
if (baseBox != null)
baseBox = ChangeWidth(baseBox, maxWidth);
baseBox = ChangeWidth(baseBox, maxWidth);
if (upperLimitBox != null)
upperLimitBox = ChangeWidth(upperLimitBox, maxWidth);
if (lowerLimitBox != null)
Expand All @@ -114,7 +113,7 @@ protected override Box CreateBoxCore(TexEnvironment environment)
}

// Add box for base atom.
resultBox.Add(baseBox!);
resultBox.Add(baseBox);

// Create and add box for lower limit.
if (this.LowerLimitAtom != null)
Expand All @@ -127,7 +126,7 @@ protected override Box CreateBoxCore(TexEnvironment environment)
}

// Adjust height and depth of result box.
var baseBoxHeight = baseBox!.Height;
var baseBoxHeight = baseBox.Height;
var totalHeight = resultBox.Height + resultBox.Depth;
if (upperLimitBox != null)
baseBoxHeight += opSpacing5 + kern + upperLimitBox.Height + upperLimitBox.Depth;
Expand Down
6 changes: 2 additions & 4 deletions src/WpfMath/Atoms/CharAtom.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ public CharAtom(SourceSpan? source, char character, string? textStyle = null)
// Null means default text style.
public string? TextStyle { get; }

private bool IsDefaultTextStyle => this.TextStyle == null;

public override ITeXFont GetStyledFont(TexEnvironment environment) =>
this.TextStyle == TexUtilities.TextStyleName ? environment.TextFont : base.GetStyledFont(environment);

protected override Result<CharInfo> GetCharInfo(ITeXFont texFont, TexStyle style) =>
this.IsDefaultTextStyle
this.TextStyle == null
? texFont.GetDefaultCharInfo(this.Character, style)
: texFont.GetCharInfo(this.Character, this.TextStyle!, style); // Nullable: CS8604, could be resolved by inlining IsDefaultTextStyle
: texFont.GetCharInfo(this.Character, this.TextStyle, style);

public override Result<CharFont> GetCharFont(ITeXFont texFont) =>
// Style is irrelevant here.
Expand Down
6 changes: 3 additions & 3 deletions src/WpfMath/Atoms/Radical.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ internal class Radical : Atom

private const double scale = 0.55;

public Radical(SourceSpan? source, Atom? baseAtom, Atom? degreeAtom = null)
public Radical(SourceSpan? source, Atom baseAtom, Atom? degreeAtom = null)
: base(source)
{
this.BaseAtom = baseAtom;
this.DegreeAtom = degreeAtom;
}

public Atom? BaseAtom { get; }
public Atom BaseAtom { get; }

public Atom? DegreeAtom { get; }

Expand All @@ -39,7 +39,7 @@ protected override Box CreateBoxCore(TexEnvironment environment)
clearance = defaultRuleThickness + Math.Abs(clearance) / 4;

// Create box for base atom, in cramped style.
var baseBox = this.BaseAtom!.CreateBox(environment.GetCrampedStyle()); // Nullable TODO: See whether BaseAtom is really always non-null here
var baseBox = this.BaseAtom.CreateBox(environment.GetCrampedStyle());

// Create box for radical sign.
var totalHeight = baseBox.Height + baseBox.Depth;
Expand Down
22 changes: 15 additions & 7 deletions src/WpfMath/Atoms/ScriptsAtom.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ protected override Box CreateBoxCore(TexEnvironment environment)
var delta = 0d;
double shiftUp, shiftDown;

if (this.BaseAtom is AccentedAtom accentedAtom)
if (this.BaseAtom is AccentedAtom accentedAtom && accentedAtom.BaseAtom != null)
{
var accentedBox = accentedAtom.BaseAtom!.CreateBox(environment.GetCrampedStyle()); // Nullable TODO: This probably needs null checking
var accentedBox = accentedAtom.BaseAtom.CreateBox(environment.GetCrampedStyle());
shiftUp = accentedBox.Height - texFont.GetSupDrop(superscriptStyle.Style);
shiftDown = accentedBox.Depth + texFont.GetSubDrop(subscriptStyle.Style);
}
Expand Down Expand Up @@ -141,16 +141,21 @@ protected override Box CreateBoxCore(TexEnvironment environment)
// Check if only superscript is set.
if (subscriptBox == null)
{
superscriptContainerBox!.Shift = -shiftUp; // Nullable TODO: Check whether superscriptContainerBox is really always non-null here
if (superscriptContainerBox == null)
throw new Exception("ScriptsAtom with neither subscript nor superscript defined");

superscriptContainerBox.Shift = -shiftUp;
resultBox.Add(superscriptContainerBox);
return resultBox;
}

// Check if only subscript is set.
if (superscriptBox == null)
{
// Nullable TODO: Check whether subscriptContainerBox is really always non-null here
subscriptContainerBox!.Shift = Math.Max(Math.Max(shiftDown, texFont.GetSub1(style)), subscriptBox.Height - 4 *
if (subscriptContainerBox == null)
throw new Exception("ScriptsAtom with neither subscript nor superscript defined");

subscriptContainerBox.Shift = Math.Max(Math.Max(shiftDown, texFont.GetSub1(style)), subscriptBox.Height - 4 *
Math.Abs(texFont.GetXHeight(style, lastFontId)) / 5);
resultBox.Add(subscriptContainerBox);
return resultBox;
Expand Down Expand Up @@ -178,11 +183,14 @@ protected override Box CreateBoxCore(TexEnvironment environment)
scriptsInterSpace = shiftUp - superscriptBox.Depth + shiftDown - subscriptBox.Height;

// Create box containing both superscript and subscript.
if (superscriptContainerBox == null || subscriptContainerBox == null)
throw new Exception($"ScriptsAtom with superscriptContainerBox = {superscriptContainerBox} and subscriptContainerBox = {subscriptContainerBox} is not supposed to be here");

var scriptsBox = new VerticalBox();
superscriptContainerBox!.Shift = delta; // Nullable TODO: Check whether superscriptContainerBox is really always non-null here
superscriptContainerBox.Shift = delta;
scriptsBox.Add(superscriptContainerBox);
scriptsBox.Add(new StrutBox(0, scriptsInterSpace, 0, 0));
scriptsBox.Add(subscriptContainerBox!);
scriptsBox.Add(subscriptContainerBox);
scriptsBox.Height = shiftUp + superscriptBox.Height;
scriptsBox.Depth = shiftDown + subscriptBox.Depth;
resultBox.Add(scriptsBox);
Expand Down
7 changes: 5 additions & 2 deletions src/WpfMath/Atoms/SymbolAtom.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@ public static SymbolAtom GetAtom(string name, SourceSpan? source) =>
TryGetAtom(name, source, out var atom) ? atom : throw new SymbolNotFoundException(name);

#if !NET452
public static bool TryGetAtom(SourceSpan name, [NotNullWhen(true)] out SymbolAtom? atom) => TryGetAtom(name.ToString(), name, out atom);
public static bool TryGetAtom(SourceSpan name, [NotNullWhen(true)] out SymbolAtom? atom)
#else
public static bool TryGetAtom(SourceSpan name, out SymbolAtom? atom) => TryGetAtom(name.ToString(), name, out atom);
public static bool TryGetAtom(SourceSpan name, out SymbolAtom? atom)
#endif
{
return TryGetAtom(name.ToString(), name, out atom);
}

public SymbolAtom(SourceSpan? source, SymbolAtom symbolAtom, TexAtomType type)
: base(source, type)
Expand Down
2 changes: 1 addition & 1 deletion src/WpfMath/DefaultTexFontParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private static void SetCharChildParsers()

public DefaultTexFontParser()
{
var doc = XDocument.Load(new System.IO.StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!)); // Nullable: CS8604: Possibly just throw if the resource is missing?
var doc = XDocument.Load(new System.IO.StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!));
this.rootElement = doc.Root;

this.parsedTextStyles = new Dictionary<string, CharFont[]>();
Expand Down
37 changes: 20 additions & 17 deletions src/WpfMath/DelimiterFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,28 @@ public static Box CreateBox(string symbol, double minHeight, TexEnvironment envi
if (extension.Bottom != null)
resultBox.Add(new CharBox(environment, extension.Bottom) { Source = source });

// Insert repeatable part multiple times until box is high enough.
var repeatBox = new CharBox(environment, extension.Repeat!) { Source = source }; // Nullable TODO: This probably needs null checking
do
if (extension.Repeat != null)
{
if (extension.Top != null && extension.Bottom != null)
// Insert repeatable part multiple times until box is high enough.
var repeatBox = new CharBox(environment, extension.Repeat) { Source = source };
do
{
resultBox.Add(1, repeatBox);
if (extension.Middle != null)
resultBox.Add(resultBox.Children.Count - 1, repeatBox);
}
else if (extension.Bottom != null)
{
resultBox.Add(0, repeatBox);
}
else
{
resultBox.Add(repeatBox);
}
} while (resultBox.Height + resultBox.Depth < minHeight);
if (extension.Top != null && extension.Bottom != null)
{
resultBox.Add(1, repeatBox);
if (extension.Middle != null)
resultBox.Add(resultBox.Children.Count - 1, repeatBox);
}
else if (extension.Bottom != null)
{
resultBox.Add(0, repeatBox);
}
else
{
resultBox.Add(repeatBox);
}
} while (resultBox.Height + resultBox.Depth < minHeight);
}

return resultBox;
}
Expand Down
2 changes: 1 addition & 1 deletion src/WpfMath/GlueSettingsParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private static void SetStyleMappings()

public GlueSettingsParser()
{
var doc = XDocument.Load(new System.IO.StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!)); // Nullable: CS8604: Possibly just throw if the resource is missing?
var doc = XDocument.Load(new System.IO.StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!));
this.rootElement = doc.Root;

this.glueTypes = new List<Glue>();
Expand Down
2 changes: 1 addition & 1 deletion src/WpfMath/Parsers/CommandParsers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public CommandContext(

internal class CommandProcessingResult
{
/// <summary>A parsed atom. May be <c>null</c>.</summary>
/// <summary>A parsed atom..</summary>
public Atom? Atom { get; }

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/WpfMath/PredefinedTexFormulaSettingsParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private static void AddToMap(IEnumerable<XElement> mapList, string[] table)

public TexPredefinedFormulaSettingsParser()
{
var doc = XDocument.Load(new System.IO.StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!)); // Nullable: CS8604: Possibly just throw if the resource is missing?
var doc = XDocument.Load(new System.IO.StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!));
this.rootElement = doc.Root;
}

Expand Down
4 changes: 3 additions & 1 deletion src/WpfMath/TexFormulaHelper.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics;
using WpfMath.Atoms;

namespace WpfMath
Expand Down Expand Up @@ -137,7 +138,8 @@ public void AddRadical(TexFormula baseFormula)

public void AddRadical(TexFormula baseFormula, TexFormula? degreeFormula)
{
this.Add(new Radical(null, baseFormula?.RootAtom, degreeFormula?.RootAtom));
Debug.Assert(baseFormula.RootAtom != null);
this.Add(new Radical(null, baseFormula.RootAtom, degreeFormula?.RootAtom));
}

public void AddOperator(string operatorFormula, string lowerLimitFormula, string upperLimitFormula)
Expand Down
27 changes: 12 additions & 15 deletions src/WpfMath/TexFormulaParser.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Windows;
Expand Down Expand Up @@ -43,9 +44,9 @@ public class TexFormulaParser
"sqrt"
};

private static IList<string> symbols = new List<string>(); // Nullable: CS8618
private static IList<string> delimeters = new List<string>();
private static HashSet<string> textStyles = new HashSet<string>();
private static readonly IList<string> symbols;
private static readonly IList<string> delimeters;
private static readonly HashSet<string> textStyles;
private static readonly IDictionary<string, Func<SourceSpan, TexFormula?>> predefinedFormulas =
new Dictionary<string, Func<SourceSpan, TexFormula?>>();

Expand All @@ -65,16 +66,6 @@ public class TexFormulaParser
};

static TexFormulaParser()
{
Initialize();
}

internal static string[][] DelimiterNames
{
get { return delimiterNames; }
}

private static void Initialize()
{
//
// If start application isn't WPF, pack isn't registered by defaultTexFontParser
Expand All @@ -95,6 +86,11 @@ private static void Initialize()
predefinedFormulasParser.Parse(predefinedFormulas);
}

internal static string[][] DelimiterNames
{
get { return delimiterNames; }
}

internal static string GetDelimeterMapping(char character)
{
try
Expand Down Expand Up @@ -535,6 +531,7 @@ private TexFormula ReadScript(
environment.CreateChildEnvironment());

source = value.Segment(start, position - start);
Debug.Assert(sqrtFormula.RootAtom != null);
return new Tuple<AtomAppendMode, Atom?>(
AtomAppendMode.Add,
new Radical(source, sqrtFormula.RootAtom, degreeFormula?.RootAtom));
Expand Down Expand Up @@ -752,7 +749,7 @@ private Atom AttachScripts(
var primesRowSource = new SourceSpan(
value.SourceName,
value.Source,
primesRowAtom.Source!.Start, // Nullable TODO: This might need null checking
primesRowAtom.Source!.Start,
position - primesRowAtom.Source.Start);
primesRowAtom = primesRowAtom.WithSource(primesRowSource);

Expand Down Expand Up @@ -803,7 +800,7 @@ private Atom AttachScripts(
var superscriptAtom = superscriptFormula?.RootAtom;
if (atom.GetRightType() == TexAtomType.BigOperator)
{
var source = value.Segment(atom.Source!.Start, position - atom.Source.Start); // Nullable TODO: This might need null checking
var source = value.Segment(atom.Source!.Start, position - atom.Source.Start);
if (atom is BigOperatorAtom typedAtom)
{
return new BigOperatorAtom(
Expand Down
4 changes: 2 additions & 2 deletions src/WpfMath/TexPredefinedFormulaParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private static Type[] GetArgumentTypes(IEnumerable<XElement> args)

public TexPredefinedFormulaParser()
{
var doc = XDocument.Load(new StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!)); // Nullable: CS8604: Possibly just throw if the resource is missing?
var doc = XDocument.Load(new StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!));
this.rootElement = doc.Root;
}

Expand Down Expand Up @@ -141,7 +141,7 @@ public override void Parse(SourceSpan source, XElement element)
var argValues = GetArgumentValues(args);

var helper = new TexFormulaHelper(formula, source);
typeof(TexFormulaHelper).GetMethod(methodName, argTypes)!.Invoke(helper, argValues); // Nullable: Hard to verify here, I guess
typeof(TexFormulaHelper).GetMethod(methodName, argTypes)!.Invoke(helper, argValues);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/WpfMath/TexSymbolParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ private static void SetTypeMappings()
public TexSymbolParser()
{
// for 3.5
var doc = XDocument.Load(new StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!)); // Nullable: CS8604: Possibly just throw if the resource is missing?
var doc = XDocument.Load(new StreamReader(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceName)!));
this.rootElement = doc.Root;
}

Expand Down
2 changes: 1 addition & 1 deletion src/WpfMath/Utils/Result.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ public Result(TValue value, Exception? error)

public Result<TProduct> Map<TProduct>(Func<TValue, TProduct> mapper) => this.IsSuccess
? Result.Ok(mapper(this.Value))
: Result.Error<TProduct>(this.Error!); // Nullable: CS8604, could be resolved by inlining IsSuccess
: Result.Error<TProduct>(this.Error!);
}
}

0 comments on commit b6582a3

Please sign in to comment.