After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726191 - Support GtkIMContext's surrounding text feature
Support GtkIMContext's surrounding text feature
Status: RESOLVED INCOMPLETE
Product: vte
Classification: Core
Component: general
unspecified
Other Linux
: Normal enhancement
: future
Assigned To: VTE Maintainers
VTE Maintainers
[see comment 10 for how to proceed]
Depends on:
Blocks:
 
 
Reported: 2014-03-12 17:33 UTC by Trung Ngo
Modified: 2020-10-06 15:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to implement the surrounding text feature (22.63 KB, patch)
2014-03-13 08:54 UTC, Trung Ngo
rejected Details | Review
patch2 (4.59 KB, patch)
2020-09-17 23:47 UTC, Takao Fujiwara
none Details | Review

Description Trung Ngo 2014-03-12 17:33:55 UTC
More and more input methods are taking advantage of the surrounding text functionality. A delete-surrounding signal handler should be as easy as faking backspace and delete keypresses.
Comment 1 André Klapper 2014-03-12 21:55:18 UTC
What is "surrounding text functionality"?
Comment 2 Trung Ngo 2014-03-13 02:02:07 UTC
It's a feature that allows input methods to retrieve and delete the text surrounding the typing carret. You can read more about it on the GtkIMContext doc page. To support it, we have to implement the "delete-surrounding" and "retrieve-surrounding" signal handlers.

https://developer.gnome.org/gtk3/stable/GtkIMContext.html
Comment 3 Trung Ngo 2014-03-13 08:54:56 UTC
Created attachment 271691 [details] [review]
Patch to implement the surrounding text feature

This patch is working as I expected. It can already allow my input method to query the text in the current terminal line, query the cursor position, delete the text to the left and right of the cursor.
Comment 4 Christian Persch 2014-03-13 11:58:02 UTC
This is too simple, since it assumes each cell contains exactly one character of the string the input method wants to delete. But with combining characters, that's not true.
Comment 5 Trung Ngo 2014-03-13 13:31:59 UTC
This patch is only sending fake backspace/delete keys so whatever VTE is doing with backspaces from user, it is doing the same.
Comment 6 Trung Ngo 2014-03-18 14:24:42 UTC
Hi, Christian. Can this patch be merged? If not, what can I do to improve it?
Comment 7 Christian Persch 2014-04-12 14:39:23 UTC
Comment on attachment 271691 [details] [review]
Patch to implement the surrounding text feature

I think what should be done is firstly that the delete-surrounding handler should check the surrounding text hasn't changed since the last retrieve-surrounding. 

Then it should convert the given (offset, n_chars) into the number of cells to delete, and issue that many delete/backspace sequences. (This still assumes that the terminal application is handling delete per cell.)

Also, do split the code refactoring from key-press handler to send_key function into a preliminary patch, and place it in vte.c so that the diff is minimised.
Comment 8 Christian Persch 2017-04-26 19:54:13 UTC
(In reply to Trung Ngo from comment #0)
> More and more input methods are taking advantage of the surrounding text
> functionality.

Got some concrete examples of ibus input methods that do use this, that I could test with?
Comment 9 Christian Persch 2018-02-17 10:52:53 UTC
No response; closing.

Please re-open if you can provide the requested information (comment 8).
Comment 10 Christian Persch 2018-04-20 17:10:20 UTC
(Just for the record, in case the requested information is ever provided and the bug reopened:)

I don't think the proposed patch takes the right approach. IMO both the 'retrieve surrounding text' and 'delete surrounding text' can never work correctly without assistance from the terminal application itself.

So what I would propose for implementation is a protocol for the terminal application to interact with the input method:

* The application enables the protocol by setting a mode (DECSET).

* When the input method wants to retrieve the surrounding text, vte sends a request sequence to the application, which will reply in a DCS sequence with the surrounding text in the data, which vte will send back to the input method.

* When the input method requests to delete the surrounding text, vte sends a sequence to the application, containing the numnber of codepoints to delete and where to delete (before/after).

And readline, at least, should then implement this protocol.
Comment 11 dcz-purism 2020-02-10 21:14:29 UTC
Hi. This is actually already supported, except in some other place which I haven't yet found. For a sample, here's the least-broken demo I managed to get on a desktop:

https://source.puri.sm/Librem5/kgx/uploads/9a7b848f381dd387a69a87c407d9eb40/input-_2_.mkv

When it works, it's much better, see my comments on the issue: https://source.puri.sm/Librem5/kgx/issues/9

If someone gives me pointers on where the functionality might be hidden, I'll try to port it to the usual im handling.
Comment 12 Christian Persch 2020-02-11 11:37:25 UTC
I can't parse what you're saying, so I don't know what you're asking.

Comment 8 asks for an actual example of an ibus input method that is using these methods. Without that, there's nothing to discuss here. Comment 4 tells you the patch proposed here isn't right; comment 7 lays out further problems with the proposed patch; and finally comment 10 tells you the patch takes an entirely wrong approach.
Comment 13 dcz-purism 2020-02-11 12:07:25 UTC
What I mean is that there exists an input method that can retrieve the surrounding text from vte, as demonstrated on the video. In the linked kgx issue, we have found out that the functionality is in the atk part after I posted here.

The part responsible for retrieving text: https://gitlab.gnome.org/GNOME/vte/blob/81c7d901f7b3e9f961cb1113251a46af6d56b783/src/vteaccess.cc#L831

Apart from being very buggy on latest versions of vte+onboard, this manages to give the input method enough info to propose convincing text suggestions, without directly tackling the problems outlined in comment 10.

Would a patch reusing this code to address comment 7 be accepted? Should one be posted here or on gitlab?
Comment 14 Christian Persch 2020-02-11 13:08:02 UTC
> as demonstrated on the video

Please always provide information in text here; don't expect me to watch random linked videos.

> What I mean is that there exists an input method that can retrieve the surrounding text from vte, as demonstrated on the video. 

What is this ibus input method's name, and where I can find the source code?

The vte code linked in comment 13 is the *a11y* code, *not* the input method code. So are you using an a11y tool (OSK, ...), or actually an input method?

> Apart from being very buggy on latest versions of vte+onboard, this manages to give

What is "vte+onboard"?

> Would a patch reusing this code to address comment 7 be accepted?

I'd be very reluctant to accept a patch that doesn't address the fundamental problem as outlined in comment 10.
Comment 15 dcz-purism 2020-02-11 13:19:32 UTC
> Please always provide information in text here; don't expect me to watch random linked videos.

Oh, don't get me wrong, I'm not particular on who watches the video. The video can be summarized as "Onboard provides suggestions for text inside gnome-terminal".

> What is this ibus input method's name, and where I can find the source code?

I have no idea whether Onboard (which is clearly shown in the video) uses ibus, but it seems to use atk. The source code can be seen here: https://bazaar.launchpad.net/~onboard/onboard/trunk/view/head:/Onboard/AtspiStateTracker.py

> What is "vte+onboard"?

It's the combination of Onboard, gnome-terminal, and the vte version. I haven't been able to make any recent one work perfectly.

> So are you using an a11y tool (OSK, ...), or actually an input method?

I don't know what distinction you have in mind. The API already implementing in vtk what this issue needs seems to be ATK is all I can say.

The reason I'm posting here is to reuse that to expose the same functionality over IMContext API. This would do nothing to address comment 10 of course, but it would not introduce any novelty into vte.
Comment 16 Christian Persch 2020-02-11 13:50:12 UTC
"Onboard" appears not to be an input method at all, and uses the a11y layer; so why would implementing surrounding text for input methods have any bearing on it?

I think what you want is make vte's a11y code implement the AtkEditableText interface so that "onboard" can use its 'delete-text' method, correct? That would seem to have the same problem as the feature requested in this bug report (comment 10).
Comment 17 dcz-purism 2020-02-11 14:14:13 UTC
The other way around, I used onboard as a proof that the functionality I'm interested in exists on vte side, while not being particularly interested in either atk or onboard.

I'm interested in making the Wayland input method to work (zwp_text_input_v3). It's implemented on the GTK side using imwayland, and vte only implements it partially by accepting submitted text, but not able to delete any (this issue), and not telling the input method what the text contents already are (this issue).

My primary goal is to make vte accept GtkIMContext's "delete-surrounding", which together with "commit" forms the minimal basis for zwp_text_input_v3. If possible, handling "retrieve-surrounding" would also be useful, but I realize that this might be a bit more difficult.

The zwp_text_input_v3 protocol is implemented by phoc https://source.puri.sm/Librem5/phoc/ and controlled using squeekboard https://source.puri.sm/Librem5/squeekboard/ . Using master versions of both, this setup is broken because backspace doesn't work. "retrieve-surrounding" is not really handled by anything, but messages it produces can be tracked using the WAYLAND_DEBUG=1 env var.
Comment 18 Christian Persch 2020-02-11 16:02:27 UTC
> [...]as a proof that the functionality I'm interested in exists on vte side[...]

But it doesn't. Vte implements neither surrounding text for im method, nor AtkEditableText. 

I'd guess that onboard just fakes some Backspace keypresses to vte, which has all the problems from comment 4 and comment 7.
Comment 19 dcz-purism 2020-02-11 16:11:25 UTC
> the functionality I'm interested in

I was imprecise there: vte implements "get some text around the cursor" usig atk, which is equal to surrounding text.

Using something derived from vte_terminal_accessible_get_text allows to check the previous characters and address comment 4 while deleting, as well as check for changes to address comment 7.

Would that kind of a delete implementation be satisfying?
Comment 20 dcz-purism 2020-02-11 16:30:00 UTC
Regarding comment 4, the GtkIMContext specifies that a delete call operates on characters and not cells, so a simple implementation should not be a problem; the complexity is supposed to be handled by the input method. From https://developer.gnome.org/gtk3/stable/GtkIMContext.html#gtk-im-context-delete-surrounding

> Note that offset and n_chars are in characters not in bytes which differs from the usage other places in GtkIMContext.
Comment 21 zbrown 2020-02-12 16:52:53 UTC
Bringing in https://gitlab.gnome.org/GNOME/vte/issues/214

im_delete_surrounding

  Simply given an offset (< 0 = before cursor, >= 0 after it) and a number

  This handles the backspace/"erase"/delete keys so is generally a simple "delete one character to the left/right" request


  I'm wondering what the best way to handle that is:

  - Feed the widget fake events?

    Arguably the "easy" solution but not very nice

  - feed_child an appropriate number of left/right & backspace/delete codes?

    Feels right, allows readline et al to do-their-thing (though I guess fake events would as well?)

  - Something else?

    I may well have missed something, what might that be? :-)


im_retrieve_surrounding

  Fairly poorly documented. We're expected to call gtk_im_context_set_surrounding in response with a piece of text and the position of the cursor within it, GtkTextView simply uses the active line for this which I guess works for us, though I failed to find a "get_cusor_row" so not sure how to proceed with that one :-)

  That said I would argue that retrieve-surrounding is a separate issue as that exists to allow contextual "suggestions" from the keyboard (or other IM) rather than being part of its basic functioning

  Of course, as you say, handling 'retrieve-surrounding' nicely would require co-operation from the child
Comment 22 Christian Persch 2020-06-18 16:18:46 UTC
Via https://gitlab.gnome.org/GNOME/vte/-/issues/214 comes the info that ibus-typing-booster [https://github.com/mike-fabian/ibus-typing-booster ?] is an ibus IM module that does use surrounding text.
Comment 23 Takao Fujiwara 2020-09-17 23:47:59 UTC
Created attachment 374287 [details] [review]
patch2

Attached here since the gitlab pull request is not open.
Comment 24 Christian Persch 2020-10-06 15:06:34 UTC
You can attach patches in gitlab as well, no need to revive closed bugs here.

This is just a rebased version of the patch that, IMO, is doing this in a completely wrong way (see comment 10).