search

gnotclub / xst

st fork that uses Xresources and some pretty good patches
MIT License
536 stars 73 forks source link

⛩ (U+26E9, shinto shrine emoji) crashes xst #70

Closed b0o closed 4 years ago

b0o commented 5 years ago

Rendering the character U+26E9 (Shinto Shrine Emoji) causes st to crash, resulting in the message:

X Error of failed request:  BadLength (poly request too large or internal Xlib length error)
  Major opcode of failed request:  139 (RENDER)
  Minor opcode of failed request:  20 (RenderAddGlyphs)
  Serial number of failed request:  4216
  Current serial number in output stream:  4241

Oddly, neither U+26E8 nor U+26EA cause a crash.

I discovered this when attempting to open an email with mutt which contained the emoji.

I've opened this issue here because my fork is based on xst, but I confirmed this to apply to suckless st (as of 21367a0) as well, so I've reported the issue to the dev@suckless.org mailing list.

$ uname -srvo
Linux 5.0.5-arch1-1-ARCH #1 SMP PREEMPT Wed Mar 27 17:53:10 UTC 2019 GNU/Linux
$ wmname
xmonad
actionless commented 5 years ago

sorry to say it, but if the issue is reproducible in st as well it doesn't make much sense to report here

you could try to get a backtrace (bt full in gdb) and try looking into what's going on

actionless commented 5 years ago

@neeasade @laserswald what do you think about strategy for st issues, should we close them, right?

neeasade commented 5 years ago

what do you think about strategy for st issues, should we close them, right?

I think so, yes, but still appreciate the report.

b0o commented 5 years ago

I just wanted to share that I worked around this in my fork by adapting a fix which suckless used in the libsl library for the same bug: https://github.com/b0o/xst/commit/808292ca0b5c2d2a1277867b40fd933e776dbc75

actionless commented 5 years ago

@b0o thanks for sharing a patch adapted for xst

@neeasade WDYT, does it makes merging it to xst? i think since we got out of sync from st codebase for quite a while it could make sense

neeasade commented 5 years ago

@actionless opting for syncing up with st codebase -- I've started on the branch sync. My plan is to keep the same file layout (st.*) and have the makefile change to have the binary/info/man files all xst still. There are ~20 conflicts to resolve atm, will PR when it's ready to test

b0o commented 5 years ago

@actionless @neeasade AFAIK this fix has not made it into st yet.

neeasade commented 5 years ago

@b0o Gotcha -- thanks for bumping this thread with your workaround! I see you've made some other nice changes too (multiple unicode code points, configurable underline/strikethrough thickness). I will cherry pick those into a PR after the we sync with upstream.

eepykate commented 4 years ago

Have you installed the symbola font? I use normal st (0.8.2) and after installing symbola it stopped crashing every time that an emoji appears on screen.

tmplt commented 4 years ago

@GaugeK Thanks, that works with xst too.

kierun commented 4 years ago

@b0o Thank you so much for the fix. Could you update your branch to include the latest from this one? Pretty please?

b0o commented 4 years ago

@kierun I don't use xst anymore, it should be pretty easy to rebase that one commit onto this repo's master.

kierun commented 4 years ago

@b0o Ah, okay. Thank you. I shall give it a go…

BTW, what do you use now? Curious mind wants to know.

b0o commented 4 years ago

@kierun I use Alacritty now mainly because I switched my WM to Sway (Wayland) and wanted to use a term with native Wayland support. I still think (x)st is an awesome terminal though.

kierun commented 4 years ago

@b0o Cool. Thank you.

BTW, the patch works fine and is indeed trivial to apply -- I have done it manually like a scrub but one could do a patch and do it properly I suppose.

I have been using (x)st and sakura but neither is quiet right. Will give Alacritty a go. Thank you.

kierun commented 4 years ago

@b0o Really off topic but I am sold on Alacritty, thank you!

neeasade commented 4 years ago

integrated @b0o's fix after a merge up to 0.8.4 - https://github.com/gnotclub/xst/commit/aa1a53d2751a6b3eeb9656652b283d75239eb70f