reutenauer / polyglossia

An alternative to Babel for XeLaTeX and LuaLaTeX
http://www.ctan.org/pkg/polyglossia
MIT License
185 stars 52 forks source link

fix polyglossia-cjk-spacing.lua #638

Closed dohyunkim closed 1 month ago

dohyunkim commented 1 month ago

There were some mistakes in last night's commit. Sorry for the inconvenience.

Udi-Fogiel commented 1 month ago

Please before loading a change run l3build check -c configfiles/config-autogen test-gloss-korean, if it fails there should be a .diff file in build/test-configfiles/config-autogen, review the changes off the log and look that they are sane.

dohyunkim commented 1 month ago

As the code was written ages ago, I forgot the reason why I treated hpack_filter and pre_linebreak_filter differently. Anyway there's no reason in this separated treatment, I have rewritten code regarding linebreaking to use pre_shaping_filter. BTW, newly introduced l3build checking system is wonderful.

Udi-Fogiel commented 1 month ago

Thanks for rewriting the code, I'll probably switch to pre_shaping_filter instead of pre_linebreak_filter and hpack_filter in the other cases we use these callbacks, now that you putt my attention on that.

Hopefully l3build will be worth the effort :)

Udi-Fogiel commented 1 month ago

@dohyunkim the log changes contains the following:

*** ./build/test-configfiles/config-autogen/test-gloss-korean.luatex.tlg    2024-05-10 12:57:22.899308165 +0300
--- ./build/test-configfiles/config-autogen/test-gloss-korean.luatex.log    2024-05-10 12:57:23.959315496 +0300
***************
*** 3,18 ****
  (gloss-korean.ldf
  File: gloss-korean.ldf polyglossia: module for Korean
  Package polyglossia Info: Option: Korean, swapstrings=all
! \xpg@attr@korean=\attribute...
  \xpg@attr@autojosa=\attribute...
! Removing  `luaotfload.node_processor' from `pre_linebreak_filter'.
! Inserting `polyglossia.lang_korean' in `pre_linebreak_filter'.
! Inserting `luaotfload.node_processor' in `pre_linebreak_filter'.
! Removing  `luaotfload.node_processor' from `hpack_filter'.
! Removing  `luaotfload.harf.finalize_hlist' from `hpack_filter'.
! Inserting `polyglossia.lang_korean' in `hpack_filter'.
! Inserting `luaotfload.node_processor' in `hpack_filter'.
! Inserting `luaotfload.harf.finalize_hlist' in `hpack_filter'.)
  Package polyglossia Info: Default language is korean
  > \box...=
  \hbox(6.66+0.22)x47.84001, direction TLT
--- 3,11 ----
  (gloss-korean.ldf
  File: gloss-korean.ldf polyglossia: module for Korean
  Package polyglossia Info: Option: Korean, swapstrings=all
! \xpg@attr@cjkspacing=\attribute...
  \xpg@attr@autojosa=\attribute...
! Inserting `polyglossia.lang_cjk_spacing' in `pre_shaping_filter'.)
  Package polyglossia Info: Default language is korean
  > \box...=
  \hbox(6.66+0.22)x47.84001, direction TLT
***************
*** 20,35 ****
--- 13,36 ----
  .\TU/lmr/m/n/10 0
  .\TU/lmr/m/n/10 1
  .\TU/lmr/m/n/10 2
+ .\penalty 50
+ .\glue 0.0 plus 0.5 minus 0.1
  .\TU/lmr/m/n/10 년
  .\TU/lmr/m/n/10 󰀀
  .\glue(\spaceskip) 3.33 plus 1.665 minus 1.11
  .\TU/lmr/m/n/10 8
+ .\penalty 50
+ .\glue 0.0 plus 0.5 minus 0.1
  .\TU/lmr/m/n/10 월
  .\TU/lmr/m/n/10 󰀀
  .\glue(\spaceskip) 3.33 plus 1.665 minus 1.11
  .\TU/lmr/m/n/10 6
+ .\penalty 50
+ .\glue 0.0 plus 0.5 minus 0.1
  .\TU/lmr/m/n/10 일
  .\TU/lmr/m/n/10 󰀀
+ .\penalty 10000
+ .\glue 0.0 plus 0.5 minus 0.1
  .\TU/lmr/m/n/10 .
  ! OK.
  <to be read again> 

I really don't like the last bit, there seems to be extra penalties and glues that were not there before. Can you check that?

dohyunkim commented 1 month ago

@Udi-Fogiel It is intended. As said before, now we treat hpack_filter as the same as pre_linebreak_filter. But the test file test-gloss-korean.lvt checks the contents of an hbox. So the diff generated.

Udi-Fogiel commented 1 month ago

Great! just wanted to check. I'll update the test.