bidi-tex / luabidi

Bidirectional typesetting with LuaLaTeX
Other
0 stars 2 forks source link

removing redundant dir nodes because of LuaTeX bug, fixing lists, cha… #12

Closed Udi-Fogiel closed 9 months ago

Udi-Fogiel commented 9 months ago

This commit should fix the package so it will be usable. allot of things are still missing, but this is a start.

jspitz commented 9 months ago

Could you give some revision log content for the docs as well? Also, some elaboration about why the fixes are needed would be good. Also, just to understand the commit message: Does this work around a current LuaTeX bug or dies it remove nodes that where there due to a former bug?

If it's a workaround, it should probably be commented more clearly as such in the code.

davidcarlisle commented 9 months ago

@Udi-Fogiel , @jspitz sorry I think I should archive this repo and not allow pull requests.

I set up bid-tex here (and arranged this to be the listed source at ctan) after @vafakhalighi contacted me and asked me to cover these packages while he would be unavailable. (and @reutenauer agreed to this one that had joint author being here)

But subsequently Vafa found time to work with TeX again and made changes in other repos (although he's welcome to have push rights here)

In particular I think this version is probably newer

https://github.com/persiantex/luabidi

I had no intention of having competing forks (and have already archived several other of the bidi-tex organisation repositories) so I'm hesitent to make changes here.

davidcarlisle commented 9 months ago

Actually @jspitz you are listed as current author/maintainer, I'm happy for you to make the call here, do you know the status of the persiantex copy?

jspitz commented 9 months ago

Hm, really? I do not see any activity (beyond an "initial commit") in Vafa's repo. My last information was that luabidi has been dropped by him (in favor of a long announced, but so far not yet visible luatex-supporting bidi package version), and we (polyglossia maintainers) have taken over for the time being, as polyglossia relies on luabidi still.

Given that @Udi-Fogiel's proposal fixes bugs and as long as there is no significant activity wrt lua + bidi, I'd rather keep this maintained.

jspitz commented 9 months ago

Hm, really?

This comment responded to @davidcarlisle's comment before the last one. Just to avoid confusion.

Udi-Fogiel commented 9 months ago

Could you give some revision log content for the docs as well?

Done.

Also, some elaboration about why the fixes are needed would be good.

In the documentation? Should I add a technical part to the documentation?

Also, just to understand the commit message: Does this work around a current LuaTeX bug or dies it remove nodes that where there due to a former bug?

It removes unnecessary dir nodes, that consequentially revealed a bug in the LuaTeX engine. Luckily they are not needed, because unlike TeX--XeT, dir nodes in LuaTeX respect grouping, so I just removed them. Broadly speaking the patch is changing the definition of \RLE from

\RLE{<Text>} -> {\@RTLtrue\pardir TRT\textdir TRT <Text>}\@RTLfalse\pardir TLT\textdir TLT

to

\RLE{<Text>} -> {\@RTLtrue\pardir TRT\textdir TRT <Text>}

It also changing more stuff that are unrelated to the bug in the engine, like removing the faulty change of \pardir, as \LRE is meant for short text, adding \leavevmode in some places so that the indentation will be on the correct side, switching \bodydir for long LTR or RTL text, so that boxes will have the correct direction (it fixes the labels of lists, for example), making sure that \pagedir is the same as \bodydir at shipout time, other wise the page can be positioned out side of the media box of the pdf, fixing lists indentation by setting the correct \shapemode (it needs to be 2 when \pagedir is different from \pardir), and setting a couple of registers that were introduced to fix bugs without breaking compatibility to 1.

Udi-Fogiel commented 9 months ago

To see the problem with lists, consider the following document:

\documentclass{article}
\usepackage{polyglossia}
\setdefaultlanguage{english}
\setotherlanguage{hebrew}
\setmainfont{Latin Modern Roman}
\newfontfamily\hebrewfont{Free Serif}

\begin{document}
Test
\begin{itemize}
    \item Test
\end{itemize}

\selectlanguage{hebrew}
Test
\begin{itemize}
    \item Test
\end{itemize}   
\end{document}

This is the output with the current version image

by adding the switches of \bodydir to \setRTL, \RTL etc. it fixes the overlapping labels, and by patching lists to use the correct \shapemode the indentation is fixed.

jspitz commented 9 months ago

Also, some elaboration about why the fixes are needed would be good.

In the documentation? Should I add a technical part to the documentation?

No, here, and maybe in the source, where appropriate. What you give now is fully sufficient. I'll push and maybe release, or do you have other things you want to do before that?

Udi-Fogiel commented 9 months ago

Also, some elaboration about why the fixes are needed would be good.

In the documentation? Should I add a technical part to the documentation?

No, here, and maybe in the source, where appropriate. What you give now is fully sufficient. I'll push and maybe release, or do you have other things you want to do before that?

Not for the current version.

jspitz commented 9 months ago

@davidcarlisle it turns out that I am only author, not maintainer on CTAN and thus cannot upload. Could you do this, please? The zip in this repo is current. Otherwise I can also ask Arthur.

davidcarlisle commented 9 months ago

@jspitz thanks I'll sort it out. I could upload to ctan although to be honest it may make more sense for me to simply ask ctan to let you upload? Whichever you prefer.

jspitz commented 9 months ago

luabidi 0.6 has been submitted to CTAN. Thanks @davidcarlisle and @Udi-Fogiel !