Fix ctrl-D #1402
Fix ctrl-D #1402
Conversation
| uint32_t newPos = nextChar(l); | ||
| uint32_t oldPos = getBufCharPos(l); | ||
| uint32_t charSize = newPos - oldPos; | ||
| memmove(l->buf + oldPos, l->buf + newPos, l->len - oldPos); | ||
| l->len -= charSize; | ||
| l->chars--; |
There was a problem hiding this comment.
@anuchak We should have a mechanism to check whether the character to delete is a utf-8 character or not.
There was a problem hiding this comment.
I've changed the name of this variable to totalUTF8Chars and the mechanism is simple, just check if total size of the character > 1.
Also I'm not sure if this variable is even required or not, because I checked DuckDB's codebase and they have no such tracking variable, don't think it is required at all.
tools/shell/linenoise.cpp
Outdated
| @@ -570,6 +570,9 @@ uint32_t linenoiseComputeRenderWidth(const char* buf, size_t len) { | |||
| } | |||
|
|
|||
| int getBufCharPos(struct linenoiseState* l) { | |||
| if (l->chars == 0) { | |||
There was a problem hiding this comment.
@anuchak I don't really understand what this function is doing? Can you add some comments here?
There was a problem hiding this comment.
This function will most likely not be required, I debugged this function and all it is doing is returning the proper position of the cursor from left.
But l->pos already holds this, that's why in DuckDB's codebase everywhere only l->pos has been used instead of calling a separate function.
I'm currently fixing this issue.
|
I've pushed some changes and they should fix the original issue that the user has raised:
So this is working properly now. But we might need to refer to DuckDB because our UTF8 handling still seems to be inadequate. |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #1402 +/- ##
==========================================
- Coverage 92.80% 92.79% -0.01%
==========================================
Files 642 641 -1
Lines 22066 22043 -23
==========================================
- Hits 20478 20455 -23
Misses 1588 1588 see 7 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Related to issue #1391
This patch only fixes the bug related to ctr-d an ascii-character. If the character is utf-8, then the bug still exists.