Skip to content

Conversation

PavelSharp
Copy link

This pull request fixes an unpleasant bug in nk_str_insert_text_char and nk_str_insert_str_char.

@sleeptightAnsiC
Copy link

sleeptightAnsiC commented Oct 19, 2025

@PavelSharp Not sure, if the code is right or not, but I just wanted to note few things:

This is how pasting żółć looks like when done on master branch
(it puts 8 characters in total which seems broken)
image
and here is the same text, but on PR branch
(puts 4 characters as expected so we have an improvement)
image

So this one bug seems fixed, however, I have some problems with your PR.
Whenever I try to paste the text into "edit box", it always resets the cursor (only visually).
It also fails to copy and re-paste the very same text. See the video:

2025-10-19_17-40-13.mp4

Note that those issues do not appear on master, only on PR branch.
So... there is either another bug, or your PR fixes and breaks something at the same time.

I'm running Nuklear/demo/sdl_renderer on Linux with Xorg

@PavelSharp
Copy link
Author

Hi, @sleeptightAnsiC!
Thank you for your comment - I really appreciate the feedback. I carefully reviewed your notes and started digging into the issue. While doing so, I actually discovered two fantastic bugs in Nuklear that (hopefully!) explain all the clipboard and UTF-8 handling problems. Let me show you what I found:

1 Cursor position issue
The problem is caused by an incorrect cursor offset in line 27194 (nk_textedit_paste).
Obviously, we want to move the cursor by the number of codepoints, not bytes.
So instead of:

state->cursor += len;

it should be:

state->cursor += glyphs;

2 Clipboard copy bug - this one is mind-blowing
Nuklear calls clip->copy only once (line 28172 in nk_do_edit), like this:

edit->clip.copy(edit->clip.userdata, text, end - begin);

The backend must provide its own implementation of this function.
But guess what? It expects the length in bytes, not in codepoints (I double-checked nk_glfw3_clipboard_copy and nk_sdl_clipboard_copy). So that’s the root cause! Luckily, it’s easy to fix.
Here’s the corrected version:

text = nk_str_at_const(&edit->string, begin, &unicode, &glyph_len);
text2 = nk_str_at_const(&edit->string, end, &unicode, &glyph_len);

if (edit->clip.copy)
    edit->clip.copy(edit->clip.userdata, text, text2 - text);

After these changes, everything starts working properly, though this is still just a draft fix.
Would you like to give it a try? 😇
As an optional step towards full unicode support, it will be useful to take another look at #829.

@sleeptightAnsiC
Copy link

sleeptightAnsiC commented Oct 19, 2025

@PavelSharp
Are you using LLM (like ChatGPT/Copilot) for writing your comments or/and patches ?
I wasn't sure, if you did when I read some of your comments on other PR few days ago,
but now I'm 99% sure you did use it on your last comment. #840 also feels LLM-generated.
Please notice that LLMs make too many mistakes, and you should not use them without clear disclosure.

@sleeptightAnsiC
Copy link

The issues that I mentioned in #841 (comment) does not occur after 3342ffc

But a possibility that you used LLM to fix those worries me a lot...

@PavelSharp
Copy link
Author

PavelSharp commented Oct 19, 2025

@sleeptightAnsiC
I may use LLMs to help phrase comments more clearly, but all code and reasoning are my own. Obviously, LLMs are very good at the task of writing in beautiful English and code reviews. This technology saves time for low-responsibility tasks. Isn't that wonderful?
Besides that, I always carefully review and verify any outputs before posting.

@sleeptightAnsiC
Copy link

The problem is that #841 (comment) fells like LLM read, found solution and responded, all on its own without human involved. I would rather read whatever you put into LLM than its pseudo-correct output.

This technology saves time for low-responsibility tasks. Isn't that wonderful?

It may save your time when writing this, but it wastes everyone's time when reading and validating it.

@RobLoach Please, be very careful when merging any of this.

{
return nk_str_insert_text_utf8(str, pos, text, len);
nk_str_insert_at_rune(str, pos, text, len);
/* TODO. Most likely, each of these functions(nk_str_insert_*, nk_str_append_*)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a TODO here. Planning on tackling it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good evening, @RobLoach and thanks for reviewing this PR.
The TODO suggests that these functions probably shouldn’t just return the length, but instead check for success before returning.
However, this is just a hypothesis, and some other functions might depend on the current behavior.
I don’t yet have enough evidence to change it, so I prefer to leave it as-is for now.
Besides, this is a very minor detail that would affect fewer than 0.01% of cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this relates to the problem of sparse documentation, and we can’t know for sure what the original developer intended.
In the future, we could try to figure it out through style, observation, and testing,
but for now I think it’s more important to focus on the critical bug that will give us a working clipboard.

@PavelSharp
Copy link
Author

PavelSharp commented Oct 19, 2025

@sleeptightAnsiC
Concerns about LLM usage in this context appear to reflect a misunderstanding of how such tools are integrated into software development and debugging processes.

When working with complex code, some of us don't just ask an LLM a single question. They use it to help examine code, test ideas, and debug iteratively. Every choice is thoroughly validated.

Basically, LLMs are used as assistants for summarizing and presenting information. They do not replace human reasoning, verification, or the rigorous search for root causes.

The technical work behind these patches: analysis, testing, and verification is entirely done by me. Saying that an "LLM-generated style" implies a lack of human expertise seriously undervalues the time and professional skill dedicated to producing these issues and PRs.

Please, be very careful when merging any of this.

Such a statement is counterproductive and does not reflect the spirit of open source software development. Instead of such actions, it would be much better to focus on reviewing the proposed PR.

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.

3 participants