@b4n requested changes on this pull request.
I don't quite like the states in-between commits (although I understand their
motivation), but the final one look really promising :+1:
Apart from my comment it looks good, and could be merged whenever we do depend
on 3.24 for real (which I don't see any problem myself, 3.24.0 is getting
pretty old by now)
> + gint y = SSM(sci, SCI_POINTYFROMPOSITION, 0, pos) + line_height;
+ GdkRectangle rect = {x, y, 0, 0};
+ GdkGravity rect_anchor = gtk_widget_get_direction(GTK_WIDGET(menu)) ==
GTK_TEXT_DIR_RTL ? GDK_GRAVITY_NORTH_EAST : GDK_GRAVITY_NORTH_WEST;
This hides the caret (and the character at that position) if there is not
enough room to put the popup below the line. It's easy enough to fix by moving
`line_height` to the rect's height instead of directly shifting its Y
coordinate (and using SOUTH gravities).
Another possible improvement would be doing something similar in the X
direction, although there's no actual issue with it. However, as is, if there
isn't enough room on the right, it would then popup on the left *of the caret*,
not under the character at the caret. It might be a nitpick, but it looks a
tiny bit off with caret in overwrite mode. Yet, I don't think the previous
code supported this.
Below is a suggestion doing both (dropping the second part is fairly
straightforward though if we want to):
```suggestion
gint y = SSM(sci, SCI_POINTYFROMPOSITION, 0, pos);
gint pos_next = sci_get_position_after(sci, pos);
gint char_width = 0;
if (sci_get_line_from_position(sci, pos_next) == line)
char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
GdkRectangle rect = {x, y, char_width, line_height};
GdkGravity rect_anchor = gtk_widget_get_direction(GTK_WIDGET(menu)) ==
GTK_TEXT_DIR_RTL ? GDK_GRAVITY_SOUTH_EAST : GDK_GRAVITY_SOUTH_WEST;
```
> @@ -1403,6 +1403,20 @@ static guint get_tag_class(const TMTag *tag)
return TM_ICON_STRUCT;
}
+/* opens menu at caret position */
+static void show_menu_at_caret(GtkMenu* menu, ScintillaObject *sci)
+{
+ GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(sci));
+ gint pos = sci_get_current_position(sci);
+ gint line = sci_get_line_from_position(sci, pos);
+ gint line_height = SSM(sci, SCI_TEXTHEIGHT, line, 0);
+ gint x = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos);
+ gint y = SSM(sci, SCI_POINTYFROMPOSITION, 0, pos) + line_height;
+ GdkRectangle rect = {x, y, 0, 0};
+ GdkGravity rect_anchor = gtk_widget_get_direction(GTK_WIDGET(menu)) ==
GTK_TEXT_DIR_RTL ? GDK_GRAVITY_NORTH_EAST : GDK_GRAVITY_NORTH_WEST;
Actually [the
documentation](https://docs.gtk.org/gtk3/method.Menu.popup_at_rect.html) states:
> Anchors should be specified under the assumption that the text direction is
> left-to-right; they will be flipped horizontally automatically if the text
> direction is right-to-left.
So the switch depending on text direction should be removed.
> + gint y = SSM(sci, SCI_POINTYFROMPOSITION, 0, pos) + line_height;
+ GdkRectangle rect = {x, y, 0, 0};
+ GdkGravity rect_anchor = gtk_widget_get_direction(GTK_WIDGET(menu)) ==
GTK_TEXT_DIR_RTL ? GDK_GRAVITY_NORTH_EAST : GDK_GRAVITY_NORTH_WEST;
+ gtk_menu_popup_at_rect(GTK_MENU(menu), window, &rect, rect_anchor,
GDK_GRAVITY_NORTH_WEST, NULL);
Making the whole thing like so:
```suggestion
gint y = SSM(sci, SCI_POINTYFROMPOSITION, 0, pos);
gint pos_next = sci_get_position_after(sci, pos);
gint char_width = 0;
if (sci_get_line_from_position(sci, pos_next) == line)
char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
GdkRectangle rect = {x, y, char_width, line_height};
gtk_menu_popup_at_rect(GTK_MENU(menu), window, &rect,
GDK_GRAVITY_SOUTH_WEST, GDK_GRAVITY_NORTH_WEST, NULL);
```
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#pullrequestreview-1233241854
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/3316/review/[email protected]>
