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

Fix removing multibyte characters in prompt, fixes #974 #2383

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix removing multibyte characters in prompt, fixes #974 #2383

wants to merge 1 commit into from

Conversation

necauqua
Copy link
Contributor

@necauqua necauqua commented Sep 7, 2018

This change does what i've said in #974, works well for me.

@actionless
Copy link
Member

bit32 is available only in lua 5.2

in lua 5.3 you can do rshift with >>, but idk how to handle that for lua 5.1

@necauqua
Copy link
Contributor Author

necauqua commented Sep 7, 2018

Oh, i see
I know about operators, but bit32 is still in 5.3 (which i have) so i thought this is good enough to be universally compatible.

I've just realized that you can just compare if byte is in range [128;192) instead, huh

@codecov
Copy link

codecov bot commented Sep 7, 2018

Codecov Report

Merging #2383 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2383      +/-   ##
==========================================
+ Coverage   83.97%   83.98%   +0.01%     
==========================================
  Files         477      477              
  Lines       32750    32746       -4     
==========================================
+ Hits        27502    27503       +1     
+ Misses       5248     5243       -5
Flag Coverage Δ
#c_code 72.58% <ø> (+0.01%) ⬆️
#functionaltests 71.93% <0%> (+0.01%) ⬆️
#lua53 87.14% <100%> (+0.01%) ⬆️
#samples 71.73% <0%> (+0.01%) ⬆️
#unittests 58.33% <100%> (+0.02%) ⬆️
Impacted Files Coverage Δ
lib/awful/prompt.lua 48.58% <100%> (+0.49%) ⬆️
property.c 77.67% <0%> (+0.46%) ⬆️

@actionless
Copy link
Member

but bit32 is still in 5.3 (which i have)

mb you got installed it from luarocks as dep for some other app?

@necauqua
Copy link
Contributor Author

necauqua commented Sep 7, 2018

luarocks said that it is installed and provided by VM or rocks_provided, idk what it means, but i guess there is really no deprecated bit32 module in Lua 5.3, yeah.
Anyway, fixed it with range check

actionless
actionless previously approved these changes Sep 7, 2018
@actionless
Copy link
Member

This change does what i've said in #974, works well for me.
@necauqua
Copy link
Contributor Author

necauqua commented Sep 7, 2018

There is always that one thing that I miss (multiple times), ugh

actionless
actionless previously approved these changes Sep 7, 2018
@necauqua
Copy link
Contributor Author

necauqua commented Sep 8, 2018

I've realized that there is also an unfixed delete
Just no cursor movement allowed (for me at least) in desktop icons prompt to notice
For delete you can also remove one byte and then remove continuation bytes in a loop, but utf-8 has the prefix code, so delete could work a bit quicker, i guess:

if byte < 128 => remove 1 byte
elseif byte < 192 => continuation byte, shoulnt get there normally, remove it(with a loop so no broken utf left)?
elseif byte < 224 => remove 2 bytes
elseif byte < 240 => remove 3 bytes
elseif byte < 248 => remove 4 bytes
else => definitely invalid utf

May i commit such a fix when i get back to my PC?

@actionless actionless dismissed their stale review September 8, 2018 13:29

above comment

@actionless
Copy link
Member

idk, mb to add some helper to gears for working with utf8 strings?

@necauqua
Copy link
Contributor Author

Update:

  • laptop broke, had to finally buy a good new one (and except that i am busy with university and so on)
  • offset calculation function works, i only have some minor problems with surrogates
  • in process of redoing that ugly if-elseif 300 or so lines in awful.prompt to be more abstract (e.g. there are duplicates and i upgraded to utf-8 only ctlr+backspace but there is still ctrl+w and so on).

And yeah, awful.prompt's cursor variable counts bytes and not codepoints (as it did) to make string.sub function useful (and not to redo awful.prompt completely) and cursor moves by calculated delta of bytes.

@actionless
Copy link
Member

@necauqua any update on this?

@necauqua
Copy link
Contributor Author

Uh, completely forgot about this, I'm sorry.
I've moved to full-time job in october and since I am also a student, I did no code for joy at home in like a month (not counting small and simple labworks for the university).
I'll try to finish this on weekend maybe

@actionless
Copy link
Member

no worries, take your time; i was just checking up for stalled PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants