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

Sre latex braille ext #1004

Merged
merged 14 commits into from
Sep 28, 2023
Merged

Sre latex braille ext #1004

merged 14 commits into from
Sep 28, 2023

Conversation

zorkow
Copy link
Member

@zorkow zorkow commented Sep 18, 2023

Properly rebased version of #986 .
Thanks to @dpvc !

Copy link
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

I have checked the PR and it does seem to work as advertised. I did mark the changes we talked about of leaving nemeth as the default rather than euro, and of adding data- before latex and itemLatex attributes.

I also suggest a change for the saveI handling that doesn't use an array (it uses the call stack for that instead). That is just a suggestion if you want to use it.

Otherwise, I'm good with the PR.

@@ -6,7 +6,7 @@ import './explorer/explorer.js';
import {MathMaps} from '#js/a11y/mathmaps.js';
import base from 'speech-rule-engine/lib/mathmaps/base.json' assert { type: 'json' };
import en from 'speech-rule-engine/lib/mathmaps/en.json' assert {type: 'json' };
import nemeth from 'speech-rule-engine/lib/mathmaps/nemeth.json' assert {type: 'json' };
import euro from 'speech-rule-engine/lib/mathmaps/euro.json' assert {type: 'json' };
Copy link
Member

Choose a reason for hiding this comment

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

As we discussed in our meeting, these need to either remain nemeth by default, or have an option that switches between the two.

ts/input/tex.ts Outdated Show resolved Hide resolved
ts/input/tex/TexParser.ts Outdated Show resolved Hide resolved
ts/input/tex/TexParser.ts Outdated Show resolved Hide resolved
ts/input/tex/TexParser.ts Outdated Show resolved Hide resolved
ts/input/tex/base/BaseItems.ts Outdated Show resolved Hide resolved
ts/input/tex/base/BaseItems.ts Outdated Show resolved Hide resolved
ts/input/tex/TexParser.ts Outdated Show resolved Hide resolved
ts/input/tex/TexParser.ts Outdated Show resolved Hide resolved
ts/input/tex/TexParser.ts Outdated Show resolved Hide resolved
@dpvc
Copy link
Member

dpvc commented Sep 19, 2023

One question I had: can you tell me the difference between the itemLatex and latex attributes?

@zorkow
Copy link
Member Author

zorkow commented Sep 26, 2023

One question I had: can you tell me the difference between the itemLatex and latex attributes?

latex contains the final latex commands. itemLatex can contain incomplete elements. It might be that we don't need the latter, but I would like to have a large enough test set before messing with it again.

@zorkow
Copy link
Member Author

zorkow commented Sep 26, 2023

I've made the required changes. PTAL.

Copy link
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the changes.

@dpvc dpvc merged commit d76621f into develop Sep 28, 2023
@dpvc dpvc deleted the sre-latex-braille-ext branch September 28, 2023 15:53
@dpvc dpvc added this to the v4.0 milestone Sep 30, 2023
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.

2 participants