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

Alternate approach to capital Greek letters #1285

Merged
merged 1 commit into from May 3, 2018

Conversation

edemaine
Copy link
Member

@edemaine edemaine commented May 3, 2018

Alternative to #1283 and #1284 for solving #1282, implementing Latin-looking capital Greek letters via symbols instead of macros, which interacts with fonts more correctly.

Visual confirmation: (the first 5 letters are Latin, and remain unchanged, while the Β's to the right are capital betas)

image

This ends up making a mapping from the capital Greek letters to the Latin equivalents in the symbols table. I think it's better than macros because of the correct interaction with fonts. In the future, we could decide to populate the font with Greek capitals for better copy/paste behavior (see #1283 discussion), but the code would be nearly identical, so this is maybe good for now.

Alternative to KaTeX#1283 and KaTeX#1284 implementing Latin-looking capital Greek
letters via symbols instead of macros, which interacts with fonts more
correctly.
@codecov
Copy link

codecov bot commented May 3, 2018

Codecov Report

Merging #1285 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1285   +/-   ##
======================================
  Coverage    82.5%   82.5%           
======================================
  Files          60      60           
  Lines        3893    3893           
  Branches      651     651           
======================================
  Hits         3212    3212           
  Misses        578     578           
  Partials      103     103
Impacted Files Coverage Δ
src/macros.js 84% <ø> (-1.28%) ⬇️
src/symbols.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2554f68...d9a793c. Read the comment docs.

@ronkok
Copy link
Collaborator

ronkok commented May 3, 2018

@edemaine You beat me to this PR by about ten minutes. I realized this was possible about an hour ago.

My apologies to you and @pzinn for wasting your time on a detour. Your approach here is much better.

@edemaine
Copy link
Member Author

edemaine commented May 3, 2018

Cool, glad you agree! I wouldn't have thought of this approach without going through all the ensuing discussion, so it's not time wasted.

Do you want to review this PR?

@ronkok
Copy link
Collaborator

ronkok commented May 3, 2018

@edemaine I don't believe that I can do a final review. I never did accept @kevinbarabash's invitation to collaborate in this repository, out of fear that my poor Git skills would cause some catastrophe. And it looks as though that invitation has now expired. So I remain a contributor.

For what it is worth, I've looked at your code and it looks perfect to me.

Copy link
Member

@ry-randall ry-randall left a comment

Choose a reason for hiding this comment

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

This LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants