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

IME: put cursor after rendered text #4937

Merged
merged 1 commit into from
Apr 8, 2022
Merged

IME: put cursor after rendered text #4937

merged 1 commit into from
Apr 8, 2022

Conversation

JingMatrix
Copy link
Contributor

The cursor position could be used for determinating the ime popup position.
For the sake of consistent (possible) popup position, we should update
cursor position after rendered text.

The cursor position could be used for determinating the ime popup position.
For the sake of consistent (possible) popup position, we should update
cursor position after rendered text.
@JingMatrix
Copy link
Contributor Author

The popup window is implemented for sway (and will be accepted later), I found this bug under sway with this pull request applied:
swaywm/sway#5890

After debugging cursor update mechinism, I confirm that we should make code change in the client side (kitty).

@page-down
Copy link
Contributor

I don't know what sway has done and can't understand what problem you are trying to fix.

CONFIRMED_PREEDDITpending
------------------[ Candidates Here (IME) ]

The left side of the IME candidate box should be aligned with the last entered pre-edit text.
So when kitty receives the pre-edit text update event, it should update the IME position first, and then draw the overlay text.

Whatever you are trying to fix, you should make sure to test it under macOS/Linux(DEs) and in different IMEs and should not break the others.

@kovidgoyal kovidgoyal merged commit fb6125a into kovidgoyal:master Apr 8, 2022
@kovidgoyal
Copy link
Owner

kovidgoyal commented Apr 8, 2022 via email

@page-down
Copy link
Contributor

In the discussion thread you mentioned someone mentioned the problem of typing at the edge of the window.
I found that when the pre-edit text exceeds the window width (e.g. at the very end), it doesn't wrap the line and only renders the last character at the end of the cursor line.

@kovidgoyal
Should we add overlay line ystart and then wrap the line?

@JingMatrix
Copy link
Contributor Author

I don't know what sway has done and can't understand what problem you are trying to fix.

CONFIRMED_PREEDDITpending
------------------[ Candidates Here (IME) ]

The left side of the IME candidate box should be aligned with the last entered pre-edit text. So when kitty receives the pre-edit text update event, it should update the IME position first, and then draw the overlay text.

Whatever you are trying to fix, you should make sure to test it under macOS/Linux(DEs) and in different IMEs and should not break the others.

Logically you are right.
However, before this change, the cursor position isn't updated before ime state updates.
In fact, the cursor position is updated in the draw pre-edit text function.

Assume sway is drawing the popup windows, it needs to know the cursor position, which is suppose to be exactly after the rendered text.
After a wm (i.e. sway here) knowing the cursor position, it could do what you describled in the ascii draw.
But if you put ime_update after pre-edit text draw, wm has no way to know the new cursor position to do his job.

@page-down
Copy link
Contributor

In my opinion, the cursor position specified by the program running in the terminal should not be changed at all when drawing an overlay line. (The program does not know about this.)

... it needs to know the cursor position ...
... wm has no way to know the new cursor position to do his job ...

The position of the input method should not be specified by the cursor position, and any attempt to lay out the input method by reading the cursor position inside the terminal is a hack.

@JingMatrix
Copy link
Contributor Author

In my opinion, the cursor position specified by the program running in the terminal should not be changed at all when drawing an overlay line. (The program does not know about this.)

Well, kitty does update the cursor position, unlike other programs under wayland as an xdg-shell.
For exmaple, typing in Zathura, Zathura will simply not deal with the cursor position.
In this case, the popup will stay still until you finish.

The position of the input method should not be specified by the cursor position, and any attempt to lay out the input method by reading the cursor position inside the terminal is a hack.

Currently, kitty updates the cursor position since pre-edit function is called.
I holds on my original idea is for the goal to keep consistent in case that when the cursor position is needed.
If not, we are in a situation that the popup windows (if managed by wm) will sometimes not be exactly after pre-edit text,
because the wm used the cursor postion dealt by last call to the pre-edit text render function.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Oct 11, 2022 via email

@page-down
Copy link
Contributor

... when the pre-edit text exceeds the window width (e.g. at the very end) ...

You mean render the overlay line over the next terminal line?

Steps to Reproduce:

  • vim in a 10-character wide window
  • Use IME to type abc at the end of the remaining three character space
  • Press d, expecting abc (overlayline does not support line wrap) and actually displaying abd.

I doubt that will work given how the overlay line tracks its position.

OK, note down as a known issue.
I think this is an edge case that is rarely encountered.

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