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 749041 - BackForwardItem should use GtkTextMark to track location
BackForwardItem should use GtkTextMark to track location
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: editor
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-06 20:52 UTC by Christian Hergert
Modified: 2017-04-17 03:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
BackForwardItem: Use GtkTextMark to track locations (6.19 KB, patch)
2017-04-14 20:36 UTC, Umang Jain
none Details | Review
BackForwardItem: Use GtkTextMark to track locations (6.57 KB, patch)
2017-04-15 05:10 UTC, Umang Jain
committed Details | Review

Description Christian Hergert 2015-05-06 20:52:30 UTC
Right now, the IdeBackForwardItem uses an IdeSourceLocation to track it's position. This is fine on startup and shutdown, but during execution, we need to also insert a GtkTextMark into the buffer when loading to represent that location.

This way, if there are changes to the buffer, we can still jump to the logical position we intended.
Comment 1 Umang Jain 2017-04-14 09:04:55 UTC
Instead of IdeSourceLocation, the position is currently tracked by IdeUri. Refer to  commit ac1b5583ea for more information.
Comment 2 Umang Jain 2017-04-14 20:36:04 UTC
Created attachment 349878 [details] [review]
BackForwardItem: Use GtkTextMark to track locations

A start in effort series for making "jumps" more accurate. Later, GtkTextMark will make us jump back/forth to logical positions.
Comment 3 Christian Hergert 2017-04-14 22:17:28 UTC
Review of attachment 349878 [details] [review]:

Couple things to look at

::: libide/history/ide-back-forward-item.c
@@ -79,0 +83,7 @@
+ide_back_forward_item_set_mark (IdeBackForwardItem *self,
+                                GtkTextMark        *mark)
+{
... 4 more ...

This should probably use a weak pointer to avoid a use-after-free.

#include "ide-macros.h"

if (ide_set_weak_pointer (&self->mark, mark))
  g_object_notify_by_pspec (G_OBJECT (self), properties [PROP_MARK]);

And in dispose or finalize:

  ide_clear_weak_pointer (&self->mark);

@@ +190,3 @@
+                       "The GtkTextMark for the location",
+                       GTK_TYPE_TEXT_MARK,
+                       (G_PARAM_READWRITE |

G_PARAM_STATIC_STRINGS and G_PARAM_EXPLICIT_NOTIFY also here.

Are you sure you want G_PARAM_CONSTRUCT_ONLY? That only allows setting during creation of the instance.
Comment 4 Umang Jain 2017-04-15 05:10:27 UTC
Created attachment 349882 [details] [review]
BackForwardItem: Use GtkTextMark to track locations

Next round of the patch taking reviews into account.
Comment 5 Christian Hergert 2017-04-17 03:40:27 UTC
Thanks! Pushed with a patch to fix gobject introspection for getters.