@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 👍

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)


In src/symbols.c:

> +	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):

⬇️ Suggested change
-	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;
+	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;

In src/symbols.c:

> @@ -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 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.


In src/symbols.c:

> +	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:

⬇️ Suggested change
-	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);
+	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, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <geany/geany/pull/3316/review/1233241854@github.com>