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

feat: \nonumber/\notag support, \tag per row of {align} #2952

Merged
merged 17 commits into from Oct 31, 2021
Merged

Conversation

edemaine
Copy link
Member

@edemaine edemaine commented Apr 22, 2021

Support \nonumber (and equivalent \notag) using a global macro \@eqnsw to track whether one occurs in each row, similar to how amsmath uses global \@eqnswtrue/\@eqnswfalse. Fix #2950.

I also added support for \tag on each row of an {align} or {align*} or similar environment, so each row is either automatically numbered (represented by true), not automatically numbered (represented by false), or has an explicit tag (represented by an array of parse nodes). Fix #2379.

This required a more general mechanism for expanding a macro (\df@tag) that doesn't actually appear in the source code. We previously did this in parseTree.js only after parsing everything, but now we need it mid-parse too. The new Parser.subparse does the job.

I also had to add a mechanism for undefining a macro value. TeX represents this as setting a macro to \relax, but undefined was already used in some places in Namespace.js so I used that.

I added basic documentation and tests.

Example:

image

The tags are center-justified instead of the correct right-justified, but that was an existing bug that we can look at in another PR.

Support `\nonumber` (and equivalent `\notag`) using a global macro
`\@eqnsw` to track whether one occurs in each row, similar to how
amsmath uses global `\@eqnswtrue`/`\@eqnswfalse`.

Fix #2950
@edemaine edemaine requested a review from ronkok April 22, 2021 00:07
@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2021

Codecov Report

Merging #2952 (9d1cad2) into main (40109f6) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2952      +/-   ##
==========================================
+ Coverage   93.46%   93.50%   +0.03%     
==========================================
  Files          89       89              
  Lines        6577     6616      +39     
  Branches     1524     1537      +13     
==========================================
+ Hits         6147     6186      +39     
  Misses        399      399              
  Partials       31       31              
Impacted Files Coverage Δ
src/parseNode.js 73.33% <ø> (ø)
src/parseTree.js 100.00% <ø> (ø)
src/Namespace.js 95.23% <100.00%> (+0.36%) ⬆️
src/Parser.js 96.00% <100.00%> (+0.09%) ⬆️
src/environments/array.js 98.03% <100.00%> (+0.12%) ⬆️

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 a59135f...9d1cad2. Read the comment docs.

@edemaine
Copy link
Member Author

Hmm, perhaps I should try to implement \tag-per-row at the same time; this should be doable with a similar (if not the same) mechanism.

@edemaine edemaine marked this pull request as draft April 22, 2021 15:08
@edemaine edemaine changed the title feat: \nonumber and \notag support feat: \nonumber/\notag support, \tag per row of {align} Apr 25, 2021
@edemaine
Copy link
Member Author

OK, I've added support for \tag in rows. This was harder than expected, but not too bad. See root post for details. Now ready for review.

@edemaine edemaine marked this pull request as ready for review April 25, 2021 14:55
@edemaine
Copy link
Member Author

edemaine commented Sep 5, 2021

I fixed the merge conflicts here, so this is ready to review again. FWIW, we've had a recent explicit request for this feature (#3183), and #2379 has 5 thumbs-up. PR #3214 also currently depends on this one. Let me know if you'd rather I split this up into multiple PRs.

Copy link
Member

@ylemkimon ylemkimon left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for the delayed review. I think subparse logic can be improved later.

src/environments/array.js Outdated Show resolved Hide resolved
src/environments/array.js Outdated Show resolved Hide resolved
src/environments/array.js Outdated Show resolved Hide resolved
src/environments/array.js Show resolved Hide resolved
src/Parser.js Outdated Show resolved Hide resolved
src/MacroExpander.js Outdated Show resolved Hide resolved
@ylemkimon ylemkimon self-assigned this Oct 31, 2021
@edemaine
Copy link
Member Author

@ylemkimon Thanks for the nice improvements! Should be ready to look at again.

@ylemkimon ylemkimon merged commit 52c4778 into main Oct 31, 2021
@ylemkimon ylemkimon deleted the nonumber branch October 31, 2021 21:40
KaTeX-bot added a commit that referenced this pull request Oct 31, 2021
## [0.15.1](v0.15.0...v0.15.1) (2021-10-31)

### Features

* \nonumber/\notag support, \tag per row of {align} ([#2952](#2952)) ([52c4778](52c4778)), closes [#2950](#2950) [#2379](#2379)
@KaTeX-bot
Copy link
Member

🎉 This PR is included in version 0.15.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Support \nonumber and \notag Enable \tag within AMS environments
4 participants