GNOME Bugzilla – Bug 719800
bookmarks: Add remove_all_bookmarks function
Last modified: 2019-03-23 20:34:12 UTC
And call it in on_document_loaded before loading the bookmarks, to ensure we don't end up with an additional bookmark in the first line after reverting a document that contained bookmarks.
Created attachment 263435 [details] [review] bookmarks: Add remove_all_bookmarks function
Related to https://bugzilla.gnome.org/show_bug.cgi?id=719425
Comment on attachment 263435 [details] [review] bookmarks: Add remove_all_bookmarks function The function should take a buffer as argument, not the view.
Review of attachment 263435 [details] [review]: Looks good. Feel free to fix the small issues and push (if you have write access to the repo). ::: plugins/bookmarks/gedit-bookmarks-plugin.c @@ +98,3 @@ static void remove_bookmark (GtkSourceBuffer *buffer, GtkTextIter *iter); static void toggle_bookmark (GtkSourceBuffer *buffer, GtkTextIter *iter); +static void remove_all_bookmarks (GeditView *view); It's simpler to move the function above the calls when possible, so the prototype is not needed. @@ +1073,3 @@ +remove_all_bookmarks (GeditView *view) +{ + GtkSourceBuffer *buf; nitpick: I would write "buffer" entirely. A general principle is to optimize the reading time, not the writing of the code, because the code is read far more often than it is written. But here buf is comprehensible too, it's not a big problem. But with lots of one-letter variables, it becomes a problem. @@ +1172,3 @@ { + /* Reverting can leave one bookmark at the start, remove it. + https://bugzilla.gnome.org/show_bug.cgi?id=719425 */ The URL is useful for a FIXME comment. Here the bug is fixed, so it is not needed. It is better to use git blame to know the commit responsible for the code, and see the commit message with the URL.
The buffer as argument is good too.
Created attachment 263438 [details] [review] bookmarks: Add remove_all_bookmarks function And call it in on_document_loaded before loading the bookmarks, to ensure we don't end up with an additional bookmark in the first line after reverting a document that contained bookmarks.
Created attachment 263442 [details] [review] bookmarks: Add remove_all_bookmarks function And call it in on_document_loaded before loading the bookmarks, to ensure we don't end up with an additional bookmark in the first line after reverting a document that contained bookmarks.
Comment on attachment 263442 [details] [review] bookmarks: Add remove_all_bookmarks function Attachment 263442 [details] pushed as 8ff5cff - bookmarks: Add remove_all_bookmarks function
Commit pushed.