Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the bra and ket commands for Dirac notation support #134

Merged
merged 4 commits into from
Jun 21, 2020

Conversation

jclapis
Copy link
Contributor

@jclapis jclapis commented Jun 20, 2020

This pull request adds the \bra{} and \ket{} commands, which are useful shorthand commands for work involving quantum mechanics and Dirac notation.

Example of a common use case:
image

Their implementation is inspired by the popular braket LaTeX package.

Copy link
Collaborator

@Happypig375 Happypig375 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional tests should also be added to CSharpMath.Rendering.Tests. Test cases are located in TestRenderingMathData.cs, run all tests once to generate a reference picture so that future changes will render correctly.

@@ -456,6 +456,12 @@ class InnerEnvironment : IEnvironment {
var operatorname = ReadString();
if (!ExpectCharacter('}')) { SetError("Expected }"); return null; }
return new LargeOperator(operatorname, null);
case "bra":
var braContents = BuildInternal(true);
return new Inner(new Boundary("〈"), braContents ?? new MathList(), new Boundary("|"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When BuildInternal returns null, an parsing error has occured. braContents and kerContents should be checked for null and return null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the latest commit.

})
);
Assert.Equal(@"\ket{i}", LaTeXParser.MathListToLaTeX(list).ToString());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional tests regarding parsing errors inside \bra and \ket should be added to TestErrors at the bottom of this file. The current implementation doesn't handle these cases correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I replicated some of the parsing error tests for powers and \sqrt.

@Happypig375
Copy link
Collaborator

The documentation states that \bra and \ket are small versions but the current implementation corresponds to \Bra and \Ket. Please adjust accordingly.

Also, a comment should be left linking to the package provided in the post.

- Changed \bra and \ket to \Bra and \Ket
- Added a comment linking back to the original LaTeX package
- Added parse error unit tests for \Bra and \Ket
@jclapis
Copy link
Contributor Author

jclapis commented Jun 20, 2020

I've made all of the requested changes. Let me know if there's anything else you need.

@Happypig375
Copy link
Collaborator

@jclapis Thank you for your contribution!

@Happypig375 Happypig375 merged commit 8cca3cc into verybadcat:master Jun 21, 2020
@Happypig375 Happypig375 added Resolution/Implemented The described enhancement or housekeeping work has been implemented. Status/5. Awaiting next release This issue has been resolved and is waiting for the next release/prerelease. labels Jun 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution/Implemented The described enhancement or housekeeping work has been implemented. Status/5. Awaiting next release This issue has been resolved and is waiting for the next release/prerelease. Type/Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants