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 706467 - gtksourcecompletioncontainer should never show horizontal scrollbar
gtksourcecompletioncontainer should never show horizontal scrollbar
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2013-08-21 08:10 UTC by Christian Hergert
Modified: 2013-08-29 19:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
calculate maximun container size based on position and screen width (2.62 KB, patch)
2013-08-24 05:07 UTC, Christian Hergert
reviewed Details | Review
completioncontainer: determine max width based on screen size. (2.69 KB, patch)
2013-08-28 23:53 UTC, Christian Hergert
reviewed Details | Review
completioncontainer: calculate maximum size based on window position. (3.50 KB, patch)
2013-08-29 19:21 UTC, Christian Hergert
none Details | Review

Description Christian Hergert 2013-08-21 08:10:48 UTC
The horizontal scrollbar on gtksourcecompletioncontainer is really a distraction since it shows up so often.

I think the window should instead grow until it has reached the edge of the screen and then ellipsize the cellrenderer markup.
Comment 1 Sébastien Wilmet 2013-08-21 13:27:10 UTC
If you try to complete C functions for a GNOME software, I can easily imagine the horizontal scrollbar to appear, indeed.

So I agree with your idea.
Comment 2 Sébastien Wilmet 2013-08-21 13:31:29 UTC
For the implementation, the CompletionContainer doesn't know the screen edge. What is possible is to set a maximum width for the container. When the completion window moves, we update this maximum width.
Comment 3 Christian Hergert 2013-08-22 20:11:22 UTC
So here is something to think about.

Consider a window with 2 gtktextview's side-by-side. You are completing on the left-side, and reading from the right-side for example code.

Should the completion window extend past the width of the parent textview potentially covering valuable text on the second textview? Or should it be contained to the size of the parent textview.

Also, what if you are completing near the end of the line. Is it better to stay contained to your textview or to align the text with the textview.

I think I lean towards just covering the second textview. Or in otherwords, staying captive to the current screen, not the parent window.
Comment 4 Paolo Borelli 2013-08-22 22:13:07 UTC
(In reply to comment #3)
> I think I lean towards just covering the second textview. Or in otherwords,
> staying captive to the current screen, not the parent window.

I tend to agree when thinking about it in abstract terms (implementation and actual testing can change my mind though ;)
Comment 5 Christian Hergert 2013-08-24 05:07:48 UTC
Created attachment 252977 [details] [review]
calculate maximun container size based on position and screen width

This is a fairly minimal patch to implement this feature.

It looks at the current position of the window and the width of the screen to calculate the maximum amount of size.

I think the reason this works okay is because of the idle callback always readjusting the completion window. Not a big fan of that, but probably fine.
Comment 6 Paolo Borelli 2013-08-25 08:03:56 UTC
the patch looks ok to me, though Sébastien is more familiar with this code so I'd like him to review the patch as well.
Comment 7 Sébastien Wilmet 2013-08-25 12:45:12 UTC
When testing the patch with test-completion, I've found a bug regarding the compact completion window (with or without the patch).

Run test-completion, place the main window on the right of the screen, and type a lot of spaces. When reaching the end of the line, the completion window doesn't have a lot of space, so instead of being aligned with the GtkTextIter (the last space on the line), it is moved on the left.

With test-completion it is not a big problem, since the natural width of the GtkTreeView doesn't change often. To better see the bug, maybe the random provider should have different proposal lengths. So we can enable only the random provider, and the width would always change.

Since the window can be moved on the left to have more space, what is the limit? If there is no limit, the maximum width is the screen width, and we move the completion window a bit on the left if needed. But there should be a limit where this situation occurs. I think a good limit is 350, the MAX_WIDTH of the container (without the patch).

To summarize:
- First, place the completion window on the GtkTextIter, compute the maximum width based on the screen edge. If the GtkTreeView has enough space, we are done.
- If the GtkTreeView doesn't have enough space, there are two situations. (1) The maximum width is above 350. In this case we add a scrollbar and we take the maximum width. (2) The maximum width is below 350. In this case, we set the maximum width as 350, we compute the size of the container (it can be below 350), and we move the completion window on the left, such as the right edge of the completion window touches the right edge of the screen.
Comment 8 Sébastien Wilmet 2013-08-25 13:44:59 UTC
Now the random provider has different proposal lengths. But it seems that the bug occurs only with the patch (on the master branch it works).
Comment 9 Sébastien Wilmet 2013-08-25 20:15:13 UTC
Review of attachment 252977 [details] [review]:

::: gtksourceview/gtksourcecompletioncontainer.c
@@ +61,3 @@
+		gdk_window_get_origin (window, &xorigin, NULL);
+		screen = gdk_window_get_screen (window);
+		return gdk_screen_get_width (screen) - xorigin;

Maybe simply return MAX (350, return_value) here.
Comment 10 Christian Hergert 2013-08-28 23:53:54 UTC
Created attachment 253461 [details] [review]
completioncontainer: determine max width based on screen size.

return MAX (max_width, UNREALIZED_MAX_WIDTH)
Comment 11 Ignacio Casal Quinteiro (nacho) 2013-08-29 06:08:05 UTC
Review of attachment 253461 [details] [review]:

Minor nitpicks.

::: gtksourceview/gtksourcecompletioncontainer.c
@@ +48,3 @@
 
+static gint
+get_max_width (GtkSourceCompletionContainer *self)

do not use self but container as in the other parts of the code

@@ +50,3 @@
+get_max_width (GtkSourceCompletionContainer *self)
+{
+	GtkWidget *toplevel;

You can move some of this variables inside the block
Comment 12 Paolo Borelli 2013-08-29 07:26:53 UTC
Review of attachment 253461 [details] [review]:

Fix up Nacho's comments and push. Thanks!
Comment 13 Sébastien Wilmet 2013-08-29 11:18:21 UTC
The problem explained in comment #7 is not completely fixed. The completion window can not touch the right edge of the screen, due to the xoffset for the text alignment.
Comment 14 Christian Hergert 2013-08-29 19:21:57 UTC
Created attachment 253545 [details] [review]
completioncontainer: calculate maximum size based on window position.

* Fixes nits.
* Fixes alignment of window near right window border.
Comment 15 Christian Hergert 2013-08-29 19:34:23 UTC
Fixed in da6c9d7e3c7583ed5efdd74f55e418875a3a7a19.