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

Use u2016 for Vert #1427

Closed
wants to merge 1 commit into from
Closed

Use u2016 for Vert #1427

wants to merge 1 commit into from

Conversation

bpiwowar
Copy link

See issue comment https://github.com/Khan/KaTeX/issues/286#issuecomment-118419984 : \Vert is associated with ∥ (u2225), which can be slanted or not, but should be with ‖ (u2016).

@khanbot
Copy link

khanbot commented Jun 13, 2018

Hey @bpiwowar,

Thanks for the PR! Mind signing our Contributor License Agreement?

When you've done so, go ahead and comment [clabot:check] and I'll check again.

Yours truly,
khanbot

@codecov
Copy link

codecov bot commented Jun 13, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1427   +/-   ##
=======================================
  Coverage   82.54%   82.54%           
=======================================
  Files          77       77           
  Lines        4388     4388           
  Branches      766      766           
=======================================
  Hits         3622     3622           
  Misses        664      664           
  Partials      102      102
Impacted Files Coverage Δ
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 4a29031...bd20794. Read the comment docs.

@ronkok
Copy link
Collaborator

ronkok commented Jun 13, 2018

@bpiwowar, You make a strong case that U+2016 should be the Unicode code point used for \lVert. Unfortunately, the KaTeX fonts do not contain a glyph at code point U+2016. So the change you have proposed to symbols.js is insufficient by itself. As submitted, this will just cause KaTeX to issue a console warning that font metrics are unavailable for "‖", and then it will render a glyph from some system font, not the KaTeX fonts.

For this to be complete, one would have to change symbols.js, as you have done, and also add a glyph to the KaTeX_Main-Regular font in the KaTex font submodule.

@ronkok
Copy link
Collaborator

ronkok commented Jun 13, 2018

@bpiwowar, If your aim to make U+2016 effective as an input character, then you could write a macro:

defineMacro("\u2016", "\\Vert");

@ylemkimon
Copy link
Member

Closing this as a won't fix. Please feel free to reopen or create a new pull request. Thank you for your contribution!

@ylemkimon ylemkimon closed this Jun 15, 2018
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.

4 participants