Addons bookmark refresh fix #284
Conversation
|
LGTM, though I didn't test. |
| typedef struct | ||
| { | ||
| AoBookmarkList *bm; | ||
| GeanyDocument *document; |
There was a problem hiding this comment.
you might want to use Document IDs as you delay the reference to the document. It probably doesn't matter much in practice though, it's highly unlikely a document is reused during the delay.
|
Otherwise, as @codebrainz, I didn't test but it LGTM |
eht16
force-pushed
the
addons_bookmark_refresh_fix
branch
from
a47cb83 to
f497d9d
Compare
|
Thanks for the feedback. I changed the document referencing to IDs as suggested. Refreshing the treeview with about 5000 markers set takes about 40-50ms with de- and reattching the model. Without detaching, refreshing takes about 60-70ms, on my machine. So, as the difference is actually not really big, I removed commit 0d130be. However, I could not notice any flickering, with or without the commit. |
| AoBookmarkListPrivate *priv = AO_BOOKMARK_LIST_GET_PRIVATE(bm); | ||
| GeanyDocument *doc = document_find_by_id(container->document_id); | ||
| ScintillaObject *sci = doc->editor->sci; |
There was a problem hiding this comment.
logic error: you dereference doc here, but check whether it's NULL on the next line.
There was a problem hiding this comment.
I'm going to try to compile this. I assume the fix here is this? (edit: all built AOK, including this change)
GeanyDocument *doc = document_find_by_id(container->document_id);
if (doc != NULL && priv->enable_bookmarklist)
{
ScintillaObject *sci = doc->editor->sci;
// etc.
|
LGTM |
eht16
force-pushed
the
addons_bookmark_refresh_fix
branch
from
e8c0dcc to
3d3df5c
Compare
|
I added eht16@e8c0dcc to add the newlines and squashed unimportant commits together. |
|
And it seems I pulled too many commits into the rebase. Can this get problematic when merging? |
|
yes, it very well might. But you should be able to fix that fairly easily to fix: check master out, pull (fast-forward, |
|
@eht16 if you're interested, I just did the second solution as you didn't seem to update this already: https://github.com/b4n/geany-plugins/commits/eht16/addons_bookmark_refresh_fix ;) |
This should delay updates on Scintilla events until the GLib main loop is idling, assuming the typing or other text modifications are finished.
eht16
force-pushed
the
addons_bookmark_refresh_fix
branch
from
3d3df5c to
0ecdc42
Compare
|
Thanks again for your help and sorry for unnecessary mistakes in code and GIT usage :(. |
Update bookmark list also on any line changes in the document
In addition to updating the bookmark list on document notebook tab change
and adding/removing line markers, also rebuild the bookmark list
when the text in the document changes with lines added or deleted.
Should fix SF bugs #129 and #39.
|
np, that's what annoying reviews are for :) |
Update bookmark list also on any line changes in the document
In addition to updating the bookmark list on document notebook tab change
and adding/removing line markers, also rebuild the bookmark list
when the text in the document changes with lines added or deleted.
Should fix SF bugs 129 and 39.
This implements some ideas mentioned in the above bug reports like updating the bookmark list
in an idle callback. Additionally, the list store is detach from the GtkTreeView to increase performance on big bookmark lists.
Maybe it is still worth to add a refresh button (or rather a context menu item to refresh) but I would try it without at first.
There is one issue left: if the
Transpose current linefeature is used, the bookmarks don't update. But this is not a problem in the plugin but rather in either Geany or Scintilla as the marker also stays at the "old" line. This should be managed separately.