xmonad-contrib icon indicating copy to clipboard operation
xmonad-contrib copied to clipboard

`X.P.Unicode`: `BS` -> `String` to support Unicode descriptions

Open PRESFIL opened this issue 2 years ago • 4 comments

This PR changes X.P.Unicode string type from ByteString to String to support Unicode in descriptions. Closes #733.

Description

X.P.Unicode allows you to work with Emoji in Unicode, but does not allow you to use non-Latin characters in the description of these Emojis. X.P.Unicode works fine, but cannot display such characters correctly.

image

See how to reproduce in #733.

Chose String instead of Text because I thought it was possible to do without an extra dependency for xmonad-contrib. The branch is editable if you know how to add support without dropping the ByteString.

Checklist

  • [ ] I've read CONTRIBUTING.md

  • [x] I've considered how to best test these changes (property, unit, manually, ...) and concluded: XXX

  • [x] I updated the CHANGES.md file

PRESFIL avatar Jul 21 '22 10:07 PRESFIL

I imagine it might have been done for performance reasons as the list of all unicode codepoints is fairly large and doing it with String can be an order of magnitude slower. Whether that's actually a problem in practice, I have no idea. If it is then depending on text would probably be fine I guess.

liskin avatar Jul 28 '22 12:07 liskin

OK, I updated CHANGES.md.

PRESFIL avatar Jul 29 '22 16:07 PRESFIL

On the performance side, I've noticed performance drops in XMonad.Prompt.Man when doing incremental searches. But there are a lot of man pages (5633 lines in prompt).

Or when using XMonad.Prompt.Zsh. But the reason is XMonad.Prompt, it's entirely writen on String base, so rewriting it is a separate issue.

PRESFIL avatar Jul 29 '22 16:07 PRESFIL

Do I have to update the changes according to master in order for this PR to be merged?

PRESFIL avatar Aug 04 '22 08:08 PRESFIL

Why not merge it?

PRESFIL avatar Aug 18 '22 15:08 PRESFIL

Sorry, seems like everyone was equally busy with other things :)

On the performance side, I've noticed performance drops in XMonad.Prompt.Man when doing incremental searches. But there are a lot of man pages (5633 lines in prompt).

Or when using XMonad.Prompt.Zsh. But the reason is XMonad.Prompt, it's entirely writen on String base, so rewriting it is a separate issue.

You have noticed these performance drops only with this patch applied? Because to me it seems like X.P.Unicode is an extra prompt, not something that interacts with X.P.Man in any way.

slotThe avatar Aug 22 '22 05:08 slotThe

I only noticed this in X.P.Man, it also does use String and lots of man pages. It doesn't related with this patch.

I think this is the topic of a separate issue.

PRESFIL avatar Aug 23 '22 11:08 PRESFIL

Thanks!

slotThe avatar Aug 24 '22 13:08 slotThe

:rocket:

PRESFIL avatar Aug 24 '22 14:08 PRESFIL