-
Notifications
You must be signed in to change notification settings - Fork 18
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
46 fixecdata calltoc1membershipwcp #47
Conversation
|
pkg/precompiles.sty
Outdated
\utt{Third argument (high):} & \col{\cmdECDATA@var@argThreeHi} \\ | ||
\utt{Third argument (low):} & \col{\cmdECDATA@var@argThreeLo} \\ | ||
\utt{Fourth argument (high):} & \col{\cmdECDATA@var@argFourHi} \\ | ||
\utt{Fourth argument (low):} & \col{\cmdECDATA@var@argFourLo} \\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need the \col{...}
stuff here, do we ? All you give it are local shorthands \locXxx
which are already under \col{...}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it would be nice to have more expressive names for the arguments, e.g.
\utt{x coord. hi} & \col{\cmdECDATA@var@xHi} \\
\utt{x coord. lo} & \col{\cmdECDATA@var@xLo} \\
\utt{y coord. hi} & \col{\cmdECDATA@var@yHi} \\
\utt{y coord. lo} & \col{\cmdECDATA@var@yLo} \\
\utt{LHS of (E) hi} & \col{\cmdECDATA@var@ySquareHi} \\
\utt{LHS of (E) lo} & \col{\cmdECDATA@var@ySquareLo} \\
\utt{RHS of (E) hi} & \col{\cmdECDATA@var@xCubePlusThreeHi} \\
\utt{RHS of (E) lo} & \col{\cmdECDATA@var@xCubePlusThreeLo} \vspace{2mm} \\
\multicolumn{2}{l}{\utt{Where (E) is } y^2 = x^3 + 3} \\ % as an example
with some explanation that the LHS and RHS refer to those of equation
pkg/precompiles.sty
Outdated
\utt{Third argument (high):} & \col{\cmdECDATA@var@argThreeHi} \\ | ||
\utt{Third argument (low):} & \col{\cmdECDATA@var@argThreeLo} \\ | ||
\utt{Fourth argument (high):} & \col{\cmdECDATA@var@argFourHi} \\ | ||
\utt{Fourth argument (low):} & \col{\cmdECDATA@var@argFourLo} \\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
argThreeHi = \locBYImHi , | ||
argThreeLo = \locBYImLo , | ||
argFourHi = \locBYReHi , | ||
argFourLo = \locBYReLo , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue here: arg1 etc .. aren't expressive enough. We need better keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. If possible I would try to convert the \callToCOneMembership
method to xkeyval
\utt{x coord. hi:} & #3 \\ | ||
\utt{x coord. lo:} & #4 \\ | ||
\utt{y coord. hi:} & #5 \\ | ||
\utt{y coord. lo:} & #6 \\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also be converted to the xkeyval
package.
yImLo = \locBYImLo , | ||
yReHi = \locBYReHi , | ||
yReLo = \locBYReLo , | ||
} | ||
\] | ||
|
||
\[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This space
\]
\[
should be removed. There are better ways to create separation if this is the goal.
This reverts commit 0649956.
No description provided.