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

Attempt to let the cursor show up in the preedit area. #7883

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ShikiSuen
Copy link

@ShikiSuen ShikiSuen commented Mar 30, 2024

This solves #7849

However, there are other following issues that need to be solved (which is not included in this PR):
#7881
#7882

Also, the display.ime.preedit() has insane values. See my first reply in the issue 7881 for more information. Once 7881 gets fixed, I believe that the modifications in this PR needs extra fixes.

P.S.: This PR also updates the gitIgnore to exclude metadata files generated by JetBrain RustRover (an IDE for Rust).

@ShikiSuen
Copy link
Author

BTW (out of topic): I suggest using "cargo fmt" in lieu of "rustfmt".
Looks like the CI on Linux gets compromised while running "rustfmt".

@ShikiSuen
Copy link
Author

P.S.: I doubt whether 199bec13 is appropriate or not, considering my insufficient familiarity with both Rust and this project.

Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

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

Just some minor things I've noticed.

@kchibisov are you able to review this PR? I'm not really familiar with IME so you're probably better suited to confirm whether this behavior is correct.

.gitignore Outdated Show resolved Hide resolved
alacritty/src/display/content.rs Show resolved Hide resolved
@ShikiSuen
Copy link
Author

ShikiSuen commented Apr 1, 2024

This PR needs is currently on hold until the following upstream issue gets resolved:
rust-windowing/winit#3617

@chrisduerr chrisduerr marked this pull request as draft April 1, 2024 12:36
@chrisduerr
Copy link
Member

This PR needs is currently on hold until the following upstream issue gets resolved

Changed it to draft to better indicate its status. Feel free to change it back once that is resolved.

@kchibisov
Copy link
Member

Regardless, I don't think it's a correct resolution, from the current logic it's intended that the cursor is hidden at the end of the input, since that's how most IMEs I've used are doing so.

However when you start moving it around, you should see the hollow block cursor. Changing it is subjective because I prefer myself the cursor hidden at the end approach, rendering e.g. Beam could be also an option, but it's sort of subjective, since once you move back it's not beam anymore, which is a bit confusing.

@chrisduerr
Copy link
Member

@kchibisov I've never used IME, but why wouldn't it just always be a beam cursor?

@jansol
Copy link
Contributor

jansol commented Apr 9, 2024

Assuming I understood the premise of this PR right, this seems to be something where different platforms have somewhat different conventions. On macOS I agree with @kchibisov that the cursor should not be shown.

Here's what macOS does with the Japanese IME in TextEdit: (typed the whole span, pressed space once to convert to kanji, then pressed shift+left 4 times to get the split shown in the screenshots, then right once to shift focus to the segment on the right)
image

No cursor, only an underline. But if you press shift-left (or right) you can shorten or extend the currently active segment of the preedit candidate to change individual segments. I think this is what the _selected_range refers to? Left and right arrows switch between segments, up and down change between suggestions (_replacement_range?). This IME only allows extending the active segment to the right (the next segment will be consumed gradually when extending over it). Rather than a cursor, the active segment is indicated by the thicker underline.

Alacritty already seems to have the right behaviour, it just does not draw the thickened underline under the active segment (when there is one - the first press on shift+left turns the whole candidate into a single active segment).

Firefox does not thicken the line, but instead indicates the active segment with a strong color and the inactive ones with a weak one, which is arguably more readable than TextEdit:
image

Here's what alacritty does on macOS right now:
image

@kchibisov
Copy link
Member

Yeah, there should be some sort of selection, which is there on e.g. linux if the cursor is not right at the end, etc.

The PR targets macOS, so demanding cursor at the end maybe some IME specific on macOS? with japanese IMEs I've never seen a cursor at the end, it's always like on a screenshot and I've sued both linux and macOS stuff.

The only place where I've seen the Beam cursor is preedit inside the ime popup, because it's like a little text area.

@kchibisov
Copy link
Member

@kchibisov I've never used IME, but why wouldn't it just always be a beam cursor?

Probably to indicate that you're not actually editing the text? Since you can not move this cursor forward even if you have words forward? So it's just underline + some highlight when moving back to indicate selection.

what we do is what 99% terminals do, so it's more or less consistent. And it's the same regardless of OS from what I've seen.

@chrisduerr
Copy link
Member

what we do is what 99% terminals do, so it's more or less consistent. And it's the same regardless of OS from what I've seen.

Feel free to close if you think the current behavior is correct and the patch is not.

@ShikiSuen
Copy link
Author

ShikiSuen commented Apr 9, 2024

Quoting @jansol 's text:

Here's what macOS does with the Japanese IME in TextEdit

Here's the behavior with default Traditional Chinese Zhuyin IME on macOS:

image

The cursor is always shown as long as the ICB (preedit) is shown.

Whether the cursor is shown or not should be commanded by the IME, and IMKTextInput client should just follow IME's orders.

cc @kchibisov

@jansol
Copy link
Contributor

jansol commented Apr 9, 2024

Here's the behavior with default Traditional Chinese Zhuyin IME on macOS:

Hm right, looks like the Pinyin IME also shows a cursor instead of a segmented underline.

@kchibisov
Copy link
Member

@ShikiSuen is this information passed somehow?

@ShikiSuen
Copy link
Author

@kchibisov

https://github.com/openvanilla/McBopomofo/blob/e537cf936bbaa796441c2672b8c369eac5a0df1b/Source/InputMethodController.swift#L446

Note that macOS tend to hide the in-ICB cursor if the ICB underline is segmented and at least 1 segment has thickened / emphasized underline. The case mentioned in this reply (McBopomofo) uses thick underline segment in Marking state, and the cursur is invisible in such case:

https://github.com/openvanilla/McBopomofo/blob/e537cf936bbaa796441c2672b8c369eac5a0df1b/Source/InputState.swift#L410-L414

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

Successfully merging this pull request may close these issues.

None yet

5 participants