Merged #3316 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#event-10658414985
You are receiving this because you are subscribed to this thread.
Message ID:
> I added a few minor changes, including a small behavioral one: the goto popup
> is again positioned at the mouse if triggered with it, which makes it
> slightly better for that use case.
Looks good (and works too from what I've tried) - though the previous version
was just fine too.
I added a few minor changes, including a small behavioral one: the goto popup
is again positioned at the mouse if triggered with it, which makes it slightly
better for that use case.
--
Reply to this email directly or view it on GitHub:
@b4n approved this pull request.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#pullrequestreview-1678848601
You are receiving this because you are subscribed to this thread.
Message ID:
@b4n pushed 3 commits.
869606ee1427b3611d01ee37c4811942241bb320 Pass the event to
gtk_menu_popup_at_pointer() when possible
705dc3c8eb39c43e54daf171811b412d6eaa5032 Use gtk_menu_popup_at_pointer() in
filebrowser as well
d10cfb503d30ee9bb7848f171dc857e2e90f0de5 Position goto popup at the
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
> I went ahead and implemented the more basic version that is similar to what
> was there but don't produce
@b4n commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
I guess I feel like the popup should be under the letter the caret is at (not
sure what are the real differences
@b4n pushed 1 commit.
3498fe12a3be953511b810e568991e4149f2613a Fix popup position on a wrapping
corner case
--
View it on GitHub:
https://github.com/geany/geany/pull/3316/files/4a2999c18e2db59bb40b90ad6453732a2596a316..3498fe12a3be953511b810e568991e4149f2613a
You are receiving this because
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
Yeah, and what's the problem here? You would like to have the popup's right
corner one character to the right?
Re-rebased on top of master to fix merge conflicts
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1755442929
You are receiving this because you are subscribed to this thread.
Message ID:
@b4n pushed 4 commits.
d906453916469769e68a2b35345813aa66737e7e Fix go to symbol definition popup
location
956409479a4dda51e95357cbb28336c121724238 Drop support for GTK 3.21 and older
bbfc201313f948f0528280a9089bc79af1af98a0 drop ui_menu_popup function
4a2999c18e2db59bb40b90ad6453732a2596a316
@b4n commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
…and for comparison, the exact same screenshot (to my freehand abilities…) with
the `SCI_POINTYFROMPOSITION`
@b4n commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
With `char_width=1` (and none of the extra tests), I get this:
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
> Yes it is
To be clear, I meant the screenshot is with the `char_width=1` version.
--
Reply to this email
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
> This is with the current state of this PR right, not with just char_width=0,
> is it?
Yes it is (to be
> > I just rebased on top of master to fix the merge conflict. I did not
> > include my latest suggested change.
>
> And I'm afraid I just introduced another merge conflict by merging #3547...
No worries, I have it resolved locally and it's not a big deal.
--
Reply to this email directly or
@b4n commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
This is with the current state of this PR right, not with just `char_width=0`,
is it?
The problem with the
> I just rebased on top of master to fix the merge conflict. I did not include
> my latest suggested change.
And I'm afraid I just introduced another merge conflict by merging #3547...
--
Reply to this email directly or view it on GitHub:
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
I don't see what the problem is in this case - this is what happens when I
invoke it on `i` and move the
@b4n commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
Ah… I feel it looks bad if cursor is in block mode (or overwrite) and the popup
is placed on the left (when there
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
> Which one do you refer to as "lazy"?
Using `char_width = 0` and not calculating it.
--
Reply to this email
@b4n commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
> But really, I'd be for the "lazy" solution here, it's not worth complicating
> the code too much I think.
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
But really, I'd be for the "lazy" solution here, it's not worth complicating
the code too much I think.
--
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
> Another possible improvement would be doing something similar in the X
> direction, although there's no
@techee commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
Maybe it was mentioned somewhere in the previous conversation but what's the
point of calculating
I just rebased on top of master to fix the merge conflict. I did *not* include
my latest suggested change.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1752161331
You are receiving this because you are subscribed to this thread.
@b4n pushed 4 commits.
c73dc3f0aedbf9b0fbc977a213dbf65b1d8a22f9 Fix go to symbol definition popup
location
229e598dd2b385d39c558b00594eed20db00f684 Drop support for GTK 3.21 and older
439aacc00596611585eeaf5b28378fef2c806bcb drop ui_menu_popup function
d2a64b0ae3af4acc3bad11230a1dec8c20e4bba5
@b4n commented on this pull request.
> + if (sci_get_line_from_position(sci, pos_next) == line)
+ char_width = SSM(sci, SCI_POINTXFROMPOSITION, 0, pos_next) - x;
As I'm a sucker for weird corner cases, this actually does not work if line
wrapping is enabled and the caret is
Yeah I think we should. There's a conflict, and I'll re-review it once more,
but it probably should make it.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1752145766
You are receiving this because you are subscribed to this thread.
This is a wayland bugfix IIUC, so yeah would be good to include in 2.0.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1751978216
You are receiving this because you are subscribed to this thread.
Message ID:
> @techee @b4n do we still want this in 2.0?
I would say so.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1751976951
You are receiving this because you are subscribed to this thread.
Message ID:
@techee @b4n do we still want this in 2.0?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1751845522
You are receiving this because you are subscribed to this thread.
Message ID:
Before a final review or ideally merge of
https://github.com/geany/geany-plugins/pull/1201 and
https://github.com/geany/geany/pull/3315 is necessary to get the
pipeline ready for GTK 3.24.
--
Reply to this email directly or view it on GitHub:
Agree, there is general agreement buried in #3178 to require 3.24, just needs
"somebody"(s) to make autotools and meson require it.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1369432930
You are receiving this because you are
I think it's pretty safe to assume that the next release will depend on 3.24
and we don't need to add new code or keep copies of old code to handle <3.24
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1369424789
You are receiving this
> 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)
Another possibility is to keep the old code and use ifdefs to pick the
implementation based on gtk 3.24 availability
@dolik-rce pushed 1 commit.
94833eaf14a3c93c70f9bab3c6d4ad63774d2adc Fix popup overlapping the carret
--
View it on GitHub:
https://github.com/geany/geany/pull/3316/files/bbbfda4679f6ca2680be1149f9f0fdcae83b21e1..94833eaf14a3c93c70f9bab3c6d4ad63774d2adc
You are receiving this because you are
@dolik-rce commented on this pull request.
> + 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 :
> I don't quite like the states in-between commits (although I understand their
> motivation)
I can squash or rearrange the commits if you want. The commits were primarily
separated to allow immediate merge, without waiting for GTK 3.24. But I believe
that it is now almost certain, that next
@b4n commented on this pull request.
> @@ -1404,90 +1404,10 @@ static guint get_tag_class(const TMTag *tag)
}
-/* positions a popup at the caret from the ScintillaObject in @p data */
-static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean
*push_in, gpointer data)
@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
@techee commented on this pull request.
> @@ -1404,90 +1404,10 @@ static guint get_tag_class(const TMTag *tag)
}
-/* positions a popup at the caret from the ScintillaObject in @p data */
-static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean
*push_in, gpointer
@dolik-rce pushed 1 commit.
bbbfda4679f6ca2680be1149f9f0fdcae83b21e1 drop ui_menu_popup function
--
View it on GitHub:
https://github.com/geany/geany/pull/3316/files/5abcbf102300671677931ec0722a5923719b8598..bbbfda4679f6ca2680be1149f9f0fdcae83b21e1
You are receiving this because you are
@dolik-rce commented on this pull request.
> {
- /* Use appropriate function for menu popup:
- - gtk_menu_popup_at_pointer is not available on GTK older than
3.22
- - gtk_menu_popup is deprecated and causes issues on
multimonitor wayland setups */
-#if
@eht16 commented on this pull request.
> @@ -1404,90 +1404,10 @@ static guint get_tag_class(const TMTag *tag)
}
-/* positions a popup at the caret from the ScintillaObject in @p data */
-static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean
*push_in, gpointer
@eht16 commented on this pull request.
> {
- /* Use appropriate function for menu popup:
- - gtk_menu_popup_at_pointer is not available on GTK older than
3.22
- - gtk_menu_popup is deprecated and causes issues on
multimonitor wayland setups */
-#if
> next line which sets button_event as data also adds gdk_event_free as a
> destructor
Oh, right. I have totally overlooked that.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3316#issuecomment-1283389879
You are receiving this because you are
@dolik-rce commented on this pull request.
> @@ -3230,14 +3230,109 @@ gboolean
> ui_encodings_combo_box_set_active_encoding(GtkComboBox *combo, gint enc
return FALSE;
}
-void ui_menu_popup(GtkMenu* menu, GtkMenuPositionFunc func, gpointer data,
guint button, guint32 activate_time)
@dolik-rce pushed 2 commits.
714748ae290d5c18d75851f7863360c42899e487 Fix go to symbol definition popup
location
5abcbf102300671677931ec0722a5923719b8598 Drop support for GTK 3.21 and older
--
View it on GitHub:
> BTW: This looks like a memory leak. The event object is only freed if it is
> triggered by keyboard. Good thing that we'll get rid of it soon
> slightly_smiling_face
No, the next line which sets `button_event ` as data also adds `gdk_event_free`
as a destructor, so its freed when the menu
@elextr commented on this pull request.
> @@ -3230,14 +3230,109 @@ gboolean
> ui_encodings_combo_box_set_active_encoding(GtkComboBox *combo, gint enc
return FALSE;
}
-void ui_menu_popup(GtkMenu* menu, GtkMenuPositionFunc func, gpointer data,
guint button, guint32 activate_time)
+
@dolik-rce commented on this pull request.
> - if (event && event->type == GDK_BUTTON_PRESS)
- button_event = (GdkEventButton *) event;
- else
- gdk_event_free(event);
BTW: This looks like a memory leak. The event object is only freed if it is
triggered
52 matches
Mail list logo
