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 318459 - entry completion should put selected text in entry
entry completion should put selected text in entry
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkComboBox
unspecified
Other Linux
: Normal enhancement
: Medium feature
Assigned To: gtk-bugs
gtk-bugs
Big patch
: 376427 (view as bug list)
Depends on:
Blocks: 102528
 
 
Reported: 2005-10-10 13:39 UTC by Christian Persch
Modified: 2007-08-29 05:13 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
add property editors to testentrycompletion (4.27 KB, patch)
2005-10-11 18:06 UTC, Christian Persch
committed Details | Review
Patch (6.92 KB, patch)
2006-12-03 09:55 UTC, Xan Lopez
none Details | Review
browser-mode (7.32 KB, patch)
2006-12-03 10:23 UTC, Xan Lopez
none Details | Review
New version of browser mode for entrycompletion. (12.63 KB, patch)
2007-02-18 14:54 UTC, Xan Lopez
none Details | Review
Latest version. (13.76 KB, patch)
2007-04-07 19:23 UTC, Xan Lopez
none Details | Review
inline selection, fixed (13.78 KB, patch)
2007-04-26 16:10 UTC, Xan Lopez
none Details | Review
inline selection, version 3 (15.31 KB, patch)
2007-04-26 19:51 UTC, Xan Lopez
none Details | Review
my version (13.22 KB, patch)
2007-04-26 22:26 UTC, Matthias Clasen
none Details | Review
Re-add completion prefix (17.68 KB, patch)
2007-04-27 20:05 UTC, Xan Lopez
none Details | Review
Remove g_return_* from the private functions. (17.53 KB, patch)
2007-04-27 20:08 UTC, Xan Lopez
none Details | Review

Description Christian Persch 2005-10-10 13:39:45 UTC
Steps to reproduce:
0) Start testentrycompletion
1) Type "g" in the first entry
2) Move down to "GNOME" entry

Expected result:
The selected entry, "GNOME" is put in the entry, without changing the displayed
completion of course, so I can go further down the list. When I type another
letter/cursor movement/backspace, the completion changes to the one appropriate
for the new text in the entry. ESC closes the completion popup and brings back
the originally entered text.

This completion mode should be supported, optionally enabled via API.
Comment 1 Matthias Clasen 2005-10-11 17:13:56 UTC
Are you asking for GtkEntryCompletion::inline-completion ?

Or do you want something different ?
Comment 2 Christian Persch 2005-10-11 18:06:02 UTC
Created attachment 53337 [details] [review]
add property editors to testentrycompletion

Something different. inline-completion only pre-enters the first completion;
but what I'm proposing is to show the completion choices for what the user has
entered so far, then when moving up/down in the completion popup, put that text
in the entry but don't change the completion yet until the user enters more
text or backspaces, deletes or ESCs. That way the user can use and modify
completed text without activating the entry.

(Oh just in case you're interested, here's a patch to add property editors for
the entry completions to the testentrycompletion program.)
Comment 3 Matthias Clasen 2005-10-26 03:57:35 UTC
Thanks for the patch. I think the requested feature will need some work,
in particular if we want to make repeated tabbing work for skipping through
the possible completions.

2005-10-25  Matthias Clasen  <mclasen@redhat.com>

        * tests/Makefile.am:
        * tests/testentrycompletion.c: Apply a patch by Christian
        Persch to add property editors.

Comment 4 Christian Persch 2006-01-06 23:28:12 UTC
Setting patch status.
Comment 5 Reinout van Schouwen 2006-02-23 18:48:50 UTC
cc'ing myself.
Comment 6 Wouter Bolsterlee (uws) 2006-06-08 12:48:23 UTC
Is this feature planned for gtk 2.10?
Comment 7 Tijmen Ruizendaal 2006-06-08 12:49:45 UTC
cc'ing myself
Comment 8 Mariano Draghi 2006-08-23 02:05:19 UTC
cc'ing myself.
Comment 9 Xan Lopez 2006-12-03 09:53:11 UTC
Hi, I'm attaching a possible implementation of this feature. It's not really robust I guess, and it lacks API to set a custom function to decide what to show in the entry for each completion item (would be very useful for epiphany). Anyway, would be nice to get some feedback on the way it is done.
Comment 10 Xan Lopez 2006-12-03 09:55:24 UTC
Created attachment 77567 [details] [review]
Patch

browser-mode is just a random name btw, suggestions welcome
Comment 11 Xan Lopez 2006-12-03 10:23:59 UTC
Created attachment 77568 [details] [review]
browser-mode

This is probably cleaner.
Comment 12 Gavin Gilmour 2006-12-08 21:31:45 UTC
cc'ing myself.
Comment 13 Reinout van Schouwen 2007-01-08 08:42:54 UTC
*** Bug 376427 has been marked as a duplicate of this bug. ***
Comment 14 Xan Lopez 2007-02-18 14:54:34 UTC
Created attachment 82819 [details] [review]
New version of browser mode for entrycompletion.

This new version adds a "cursor-on-match" signal that applications can connect to in order to customize the action performed on each completion selection. See bug 409291 for an example. The nomenclature is still confusing. I think the signal should be named "selection-on-match", and "match-selected" should be renamed to "match-activated", but it's obviously too late for this. Suggestions welcome.
Comment 15 Matthias Clasen 2007-02-26 20:48:14 UTC
Some comments:

1) A better name than browser-mode would be nice. I don't have a proposal right now.

2) It would be nice if the "tentative" text that gets removed on ESC would be selected (like inline completion does)

3) there is some interaction with inline completion where using up/down to browse through matches accepts an outstanding completion that was inserted by inline completion.
Comment 16 Xan Lopez 2007-02-26 21:13:47 UTC
(In reply to comment #15)
> Some comments:
> 
> 1) A better name than browser-mode would be nice. I don't have a proposal right
> now.
> 

Yeah... Something stupid like "completion-on-entry" or "completion-pre-insert" might do the trick, but *shrug*.

> 2) It would be nice if the "tentative" text that gets removed on ESC would be
> selected (like inline completion does)

Agreed, will do this.

> 
> 3) there is some interaction with inline completion where using up/down to
> browse through matches accepts an outstanding completion that was inserted by
> inline completion.
>

I'd call this a feature, but probably that's just me. We can live with this and document it or I can try to figure out it can be avoided.

About cursor-on-match being redundant (as commented on IRC), match-selected has the semantics of the item being "activated" (either with Enter or button press), so for example Epiphany will load the item referenced by the callback parameters. I don't think pre-insertion on the entry can be done with the same signal in a sane way. 

Comment 17 Xan Lopez 2007-03-13 20:19:48 UTC
Any feedback on the two last comments? I have point 2) sitting in my tree waiting on the feedback of the other comments :)

Btw, I'd also like to see bug 309035 fixed. Is some kind of generic access as proposed by chpe something you'd like to add to GTK+? If we add it, should this go anyway? Probably yes, as it's something generally useful and frequently requested...
Comment 18 Raphael Bosshard 2007-03-14 21:36:06 UTC
Oh, yes, please! Let's go ahead and put this into the upstream-tree. This is the only thing that realy bothers me in Ephiphany. I've even started 'programming' a somewhat working Epiphany-extension. But having this in gtk+ would be soooo much better. :-D
Comment 19 Matthias Clasen 2007-03-23 05:01:40 UTC
How is "inline-selection" as a name ? That seems to describe what this does, showing the current selection inline.

Wrt to the interaction between inline-completion and inline-selection, you could
probably remove any inline-completion inserted prefix before emitting cursor-on-match.
Comment 20 Xan Lopez 2007-04-07 19:22:42 UTC
Wouldn't it make more sense to simply disable inline-completion if we are just going to undo its actions? Although I must say I don't see the point in this. Both things are off by default, so if someone really wants to have them at the same time (and it actually works) I don't see the reason for not allowing it.
Comment 21 Xan Lopez 2007-04-07 19:23:33 UTC
Created attachment 85962 [details] [review]
Latest version.
Comment 22 Reinout van Schouwen 2007-04-25 22:59:04 UTC
@Matthias, could you spare a moment for review of patch v4?
Comment 23 Matthias Clasen 2007-04-26 04:30:24 UTC
Xan, I don't know what git does differently here, but patch complains about the patch being malformed.
Comment 24 Steve Frécinaux 2007-04-26 08:36:15 UTC
You have to use patch -p1 < $file
Comment 25 Matthias Clasen 2007-04-26 13:23:32 UTC
I do know the patch commandline...

[mclasen@localhost trunk]$ patch -p1 < ~/Desktop/xan-completion.patch 
patching file gtk/gtkentry.c
patch: **** malformed patch at line 47: @@ -5721,6 +5751,12 @@ gtk_entry_completion_key_press (GtkWidget   *widget,

Comment 26 Xan Lopez 2007-04-26 16:10:11 UTC
Created attachment 87073 [details] [review]
inline selection, fixed

No clue what happened, as far as I can remember I extracted the diff the same way I always do... Attaching again, this one works :)
Comment 27 Matthias Clasen 2007-04-26 16:51:12 UTC
Feedback given on irc:

- would be nice if inline-selection could work without popup-completion, but thats
  not a blocker

- the behaviour when arrowing left/right in the presence of a tentative selection 
  is confusing: the selection is removed, but it remains tentative, and the popup
  stays around, too.

- there is some forgotten check, when turning on inline-selection, but not inline-completion, I get 

(lt-testentrycompletion:27362): GLib-GObject-WARNING **: gsignal.c:1718: instance `0x9a69000' has no handler with id `392'
Comment 28 Xan Lopez 2007-04-26 19:50:49 UTC
(In reply to comment #27)
> Feedback given on irc:
> 
> - would be nice if inline-selection could work without popup-completion, but
> thats
>   not a blocker

Agreed, will look at it.

> 
> - the behaviour when arrowing left/right in the presence of a tentative
> selection 
>   is confusing: the selection is removed, but it remains tentative, and the
> popup
>   stays around, too.

Fixed it by adding Left and Right as acknowledged ways of exiting the completion.

> 
> - there is some forgotten check, when turning on inline-selection, but not
> inline-completion, I get 
> 
> (lt-testentrycompletion:27362): GLib-GObject-WARNING **: gsignal.c:1718:
> instance `0x9a69000' has no handler with id `392'

The signal ids need to be reset to 0 in the disconnect_completion_signals function, the bug was already there but was never triggered.

Attaching patch.

Comment 29 Xan Lopez 2007-04-26 19:51:31 UTC
Created attachment 87092 [details] [review]
inline selection, version 3
Comment 30 Matthias Clasen 2007-04-26 21:41:05 UTC
I think it would be more nicer if Left/Right would accept the current selection for inline-selection, like they do for inline-completion. 

+    Determines wheter the possible completions on the popup
+   * will appear in the entry as you navigate thru them.

Two typos here.

Other than that, the patch looks good to me
Comment 31 Matthias Clasen 2007-04-26 22:26:53 UTC
Created attachment 87104 [details] [review]
my version

Here is my version of the patch which does accept the selection on Left/Right,
and does away with the need to keep a copy of the prefix.
Comment 32 Xan Lopez 2007-04-27 07:53:56 UTC
(In reply to comment #31)
> Created an attachment (id=87104) [edit]
> my version
> 
> Here is my version of the patch which does accept the selection on Left/Right,
> and does away with the need to keep a copy of the prefix. 
> 

Very nice solution :)

Should I commit then with the two typos fixed?
Comment 33 Matthias Clasen 2007-04-27 13:38:49 UTC
Yes, go ahead please. 
Comment 34 Xan Lopez 2007-04-27 16:51:32 UTC
2007-04-27  Xan Lopez  <xan@gnome.org>

	Support inline-selection in entries (#318459)
	
	* gtk/gtkentry.c:
	* gtk/gtkentrycompletion.c:
	* gtk/gtkentrycompletion.h:
	* gtk/gtkentryprivate.h:

	When enabled cursor-match is emited when the cursor is on
	a possible completion on the list. The default implementation
	will replace the contents on the entry with the contents of
	the text column in the completion model.

	Review and improvements by Matthias Clasen.
Comment 35 Gavin Gilmour 2007-04-27 17:35:45 UTC
This is a great day.

Thanks Xan, Matthias for making this happen!
Comment 36 Xan Lopez 2007-04-27 20:05:43 UTC
Created attachment 87167 [details] [review]
Re-add completion prefix
Comment 37 Xan Lopez 2007-04-27 20:08:20 UTC
Created attachment 87168 [details] [review]
Remove g_return_* from the private functions.
Comment 38 Matthias Clasen 2007-04-27 21:41:34 UTC
Can you give an example why the extra complication with the needle and the haystack is actually necessary ? 

Since you are passing -1, g_strstr_len is unnecessary anyway, and a simple
strstr will do.
Comment 39 Xan Lopez 2007-04-27 22:04:57 UTC
For the tentative selection to take into account or not the inline-completion. If inline-completion is enabled the added text won't be marked as tentative.
Comment 40 Xan Lopez 2007-04-27 22:44:20 UTC
The added text coming from the inline-completion, of course :)
Of course it's perfectly possible that the same thing can be done in a more straightforward way or maybe it's not even desirable in the first place.
I've also made it so that pressing right will move the position to the end of the text and left to the beginning. Felt like the right thing to do, but it's just a small detail.
Comment 41 Matthias Clasen 2007-04-27 23:01:39 UTC
Looks ok, please commit.
Comment 42 Xan Lopez 2007-04-28 07:49:57 UTC
2007-04-28  Xan Lopez  <xan@gnome.org>

	* gtk/gtkentry.c:
	* gtk/gtkentrycompletion.c:
	* gtk/gtkentrycompletion.h:
	* gtk/gtkentryprivate.h:

	Remember the user input that triggered the completion, add
	API to the retrieve it and reset the entry contents to it
	if the user cancels the tentative completion during
	the inline-selection.
Comment 43 Hongzheng Wang 2007-08-29 05:13:21 UTC
cc'ing myself.