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

\@protected@testopt definition from latex.ltx, allows loading ucs.sty #983

Merged
merged 1 commit into from May 2, 2018

Conversation

@dginev
Copy link
Collaborator

@dginev dginev commented May 2, 2018

Fixes #982

There was a rather nasty infinite loop when loading ucs.sty with the --includestyles option, which kept allocating new memory.

Turns out using the native definition of \@protected@testopt from latex.ltx allows for the package to load completely smoothly and error-free. So I'm submitting this PR to address the original report on the mailing list.

That said, looking at the documentation of ucs:
https://mirrors.sorengard.com/ctan/macros/latex/contrib/ucs/ucs.pdf

I am tempted to add a trivial binding for latexml, that sets the input encoding to utf8. Will wait for feedback from @brucemiller

@brucemiller brucemiller merged commit fd566f1 into brucemiller:master May 2, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brucemiller
Copy link
Owner

@brucemiller brucemiller commented May 2, 2018

Patch looks reasonable.

But, taking a very quick glance at ucs's docs, I'm not sure that would do it. They suggest \usepackage[utf8x}{inputenc}, so shouldn't the encoding already be utf8? (or do we need a utf8x.def binding?)

@dginev
Copy link
Collaborator Author

@dginev dginev commented May 2, 2018

Right, the patch makes the raw interpretation possible, but it does not make it correct.

Adding a ucs binding that is a simple alias for utf8 inputenc with empty stubs for all ucs-specific macros (since they may be used by the authors) could be a decent solution.

I can't say much more without actual tests, but will ask the original reporter in the email thread.

@dginev
Copy link
Collaborator Author

@dginev dginev commented May 2, 2018

Ah, I just realized I shouldn't have asked the reporter a second too late. The log suggests ucs is auto-loaded when using inputenc with --includestyles on, as seen at:


(Loading /opt/local/lib/perl5/vendor_perl/5.26/LaTeXML/Package/inputenc.sty.ltxml...
(Processing definitions /usr/local/texlive/2016/texmf-dist/tex/latex/ucs/utf8x.def... 0.15 sec) 0.54 sec)
(Processing definitions /usr/local/texlive/2016/texmf-dist/tex/latex/ucs/ucs.sty...

So I'm now agreeing with your remark about ut8x.def needing a binding that prevents further inputs.

@brucemiller
Copy link
Owner

@brucemiller brucemiller commented May 2, 2018

hmm... how's utf8x.def compare to utf8.def ?

@dginev
Copy link
Collaborator Author

@dginev dginev commented May 3, 2018

significantly? I don't understand either in sufficient depth.

The extended Unicode is something I have only loaded to get full arabic support, as the usual utf8 has some gaps. However, it is discouraged, as you run into subtle conflicts with modern packages, see e.g.
https://tex.stackexchange.com/questions/13067/utf8x-vs-utf8-inputenc

So it's probably not worth spending any significant time on it beyond parity with the regular utf8, since latexml should already offer a larger level of support as far as unicode is concerned.

@dginev
Copy link
Collaborator Author

@dginev dginev commented May 3, 2018

So I would suggest as the easiest patch to adjust SetInputEncoding in inputenc.sty.ltxml to map utf8x to utf8, and proceed as usual.

@brucemiller
Copy link
Owner

@brucemiller brucemiller commented May 3, 2018

@dginev dginev deleted the dginev:ucs-sty-fix branch Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants