[WIP] Tidy MathListIndex by charlesroddie · Pull Request #159 · verybadcat/CSharpMath

charlesroddie · 2020-08-11T09:33:16Z

Currently MathListIndex is supposed to have a SubIndex if an only if SubIndexType <> MathListSubIndexType.None. (Can you confirm that @Happypig375 ?)

This PR brings this constraint into the type structure, removing MathListSubIndexType.None and having:

public (MathListSubIndexType SubIndexType, MathListIndex SubIndex)? SubIndexInfo;

CSharpMath.Editor/MathKeyboard.cs

Happypig375 · 2020-08-11T13:36:40Z

Yes.

charlesroddie · 2020-08-12T08:28:40Z

Getting errors I'm having trouble diagnosing.

Inputs: 1, power, 2

Debug code gives:

InsertAndAdvance index before: [0, Superscript:0]
InsertAndAdvance index before: [0]
InsertAtAtomIndexAndAdvance atom: Number 2
InsertAtAtomIndexAndAdvance advance before: [0]
InsertAtAtomIndexAndAdvance advance after: [1]
InsertAndAdvance index at end: [1]
InsertAndAdvance index at end: [0, Superscript:0] // should be [0, Superscript:1]

I have the feeling it's to do with mutability and the refs. I didn't consider the refs when editing.

static void InsertAtAtomIndexAndAdvance(this MathList self, int atomIndex, MathAtom atom, ref MathListIndex advance

Independently of getting this PR working, it's good to remove the refs and make MathList immutable. This would involve more recursive code. But it could be good to get this PR working before tackling that I think.

Once this PR goes in and the immutable change we will have a clean base to work on #117 .

Any thoughts @Happypig375 ?

Happypig375 · 2020-08-12T17:49:03Z

.editorconfig

 # https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference#c-formatting-settings
 csharp_new_line_before_catch = false
-csharp_new_line_before_else = false
+#csharp_new_line_before_else = false


Happypig375 · 2020-08-12T17:49:12Z

.editorconfig

 csharp_new_line_before_members_in_object_initializers = false
 csharp_new_line_before_members_in_anonymous_types = false
-csharp_new_line_before_open_brace = none
+#csharp_new_line_before_open_brace = none


Happypig375 · 2020-08-12T17:52:25Z

CSharpMath.Editor/Extensions/MathList.cs

-        case MathListSubIndexType.Superscript:
-          self.Atoms[index.AtomIndex].Superscript.InsertAndAdvance(ref index.SubIndex, atom, advanceType);
+        case (MathListSubIndexType.Superscript, MathListIndex subIndex):
+          self.Atoms[index.AtomIndex].Superscript.InsertAndAdvance(ref subIndex, atom, advanceType);


The ref now mutates the local instead of the index. Same below.

Happypig375 · 2020-08-12T17:54:57Z

CSharpMath.Editor/SubIndexTypeMismatchException.cs

-      Array.IndexOf(typeof(MathListSubIndexType).GetEnumValues(), index.SubIndexType) == -1
-      ? $"{index.SubIndexType} is an invalid subindex type."
-      : $"{index.SubIndexType} not found at index {index.AtomIndex}.") { }
+      Array.IndexOf(typeof(MathListSubIndexType).GetEnumValues(), index.SubIndexInfo!.Value.SubIndexType) == -1


Exception will appear when SubIndexInfo is null

Happypig375 · 2020-08-12T17:55:14Z

NuGet.Config

+  <packageSources>
+    <add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" />
+  </packageSources>
+</configuration>


Happypig375 · 2020-08-12T17:56:14Z

CSharpMath.Editor/MathListIndex.cs

+
+    /// <summary>Factory function to create a `MathListIndex` with no subindexes.</summary>
+    /// <param name="index">The index of the atom that the `MathListIndex` points at.</param>
+    public static MathListIndex Level0Index(int index) => new MathListIndex(index, null);


Can be made into a default parameter of constructor

Happypig375 · 2020-08-12T17:58:15Z

Independently of getting this PR working, it's good to remove the refs

I don't think removing refs can be independent of this PR. Currently InsertAtAtomIndexAndAdvance advances the advance parameter and now that behaviour is gone, hence test failures.

charlesroddie · 2020-08-15T19:45:36Z

CSharpMath.Editor/Extensions/MathList.cs

+          return self.InsertAtAtomIndexAndAdvance(atomIndex, atom, advanceType);
+        case (MathListSubIndexType type, MathListIndex subIndex):
+          switch (type) {
+            case MathListSubIndexType.BetweenBaseAndScripts: // TODO: finish this section


I don't really understand this case. The docs on BetweenBaseAndScripts has The position in the subindex is an index into the nucleus, must be 1 and I don't understand that exactly. Could you help with this part @Happypig375 ?

The AtomIndex of the SubIndex must be 1 for SubIndexType BetweenBaseAndScripts.

I got that far...

In 12^3, is there a MathListIndex of 2?

Yes. The right boundary of the whole MathList. (for insertions)

What is the MathListIndex of 2 in 12^3?

Yes. The right boundary of the whole MathList. (for insertions)

Cursor position MathListIndex

‸12^3 0

1‸2^3 1

12‸^3 1, BetweenBaseAndScripts:1

12^‸3 1, Superscript:0

12^3‸ 1, Superscript:1 (to the right of 3) OR 2 (to the right of the entire MathList)

Tidy MathListIndex

45df37d

charlesroddie changed the title ~~Tidy MathListIndex~~ [WIP] Tidy MathListIndex Aug 11, 2020

charlesroddie added 3 commits August 11, 2020 12:36

builds

ea1dacf

fix

1624b51

tidy

d0629bf

charlesroddie commented Aug 11, 2020

View reviewed changes

CSharpMath.Editor/MathKeyboard.cs Outdated Show resolved Hide resolved

charlesroddie added 4 commits August 11, 2020 13:05

tidy

a694610

reduce diff

c2a2fbe

tidy

53650f0

reduce diff

82f4a77

charlesroddie added 2 commits August 12, 2020 08:54

tidy

f4b0271

Add debug code

24c8a5f

Happypig375 reviewed Aug 12, 2020

View reviewed changes

charlesroddie added 6 commits August 12, 2020 20:42

Documentation

af39ff0

wip

d319df6

wip

7b38420

fix

b2cd1ac

TODO

4e0998c

readonly props

3936e72

charlesroddie commented Aug 15, 2020

View reviewed changes

Happypig375 added the Type/Housekeeping This includes internal only changes. label Oct 27, 2020

Cursor position	MathListIndex
‸12^3	0
1‸2^3	1
12‸^3	1, BetweenBaseAndScripts:1
12^‸3	1, Superscript:0
12^3‸	1, Superscript:1 (to the right of 3) OR 2 (to the right of the entire MathList)

Conversation

charlesroddie commented Aug 11, 2020

Uh oh!

Uh oh!

Happypig375 commented Aug 11, 2020

Uh oh!

charlesroddie commented Aug 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Happypig375 commented Aug 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Happypig375 Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Happypig375 Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlesroddie Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Happypig375 Aug 16, 2020 •

edited

Loading

Happypig375 Aug 16, 2020 •

edited

Loading

charlesroddie Aug 16, 2020 •

edited

Loading