A commit that I introduced earlier caused crashes and I had to revert. The reason was because I used a dangling pointer. It did not occur to me that the pointer could be dangling. I just assumed it would not be (I've learned my lesson). The reason why it becomes dangling is because I add code after a dispatch and thus the buffer view might have been destroyed (e.g. from closing a buffer or closing a view).
My solution is to simply refresh the pointer (I can get it from currentBufferView()). However, this got me thinking: shouldn't we set the pointer to null every time we know that it was destroyed? What if we don't use that pointer again (as was the case before my commit)? Looking back, it is pretty obvious that the pointer could have become dangling and I should have realized this potential problem. I guess I have two questions: (1) Are there any comments on my patch? (2) Even if I did not want to add code to GuiView::dispatch, would it have been a reasonable patch to _only_ refresh the pointer after the dispatch, just in case someone wanted to add code to it eventually? Scott
From 579b970af9fd4513d2361b372491e9b0266f9862 Mon Sep 17 00:00:00 2001 From: Scott Kostyshak <skost...@lyx.org> Date: Tue, 21 Oct 2014 01:45:31 -0400 Subject: [PATCH] Recommit clearing of empty selections in GuiView A similar fix was reverted (453ce611) because of crashes. The crashes occurred simply because of a failed check that we have a buffer view before using it. That is now done in this commit. The below commit description is copied from the original commit (fb05011a): Empty selections can cause confusing behavior for a few reasons: (1) some functions behave differently depending on whether there is a selection. If I press delete, nothing happens (where I expect the character or inset before the cusor to be deleted). If I toggle bold or emphasize nothing happens (where if there is no selection the entire word is toggled). There are other LyX functions that depend on whether there is a selection or not. Further, I wonder if any part of LyX's code assumes that if there is a selection it is non-empty. (2) menu options are incorrectly set. For example, the scissors icon. For remaining empty selection issues, see #9222. For more information, see: https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg184758.html --- src/frontends/qt4/GuiView.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp index b690ac1..ad00659 100644 --- a/src/frontends/qt4/GuiView.cpp +++ b/src/frontends/qt4/GuiView.cpp @@ -3881,6 +3881,18 @@ void GuiView::dispatch(FuncRequest const & cmd, DispatchResult & dr) if (menuBar()->isVisible() && lyxrc.full_screen_menubar) menuBar()->hide(); } + + // Need to update bv because many LFUNs here might have destroyed it + bv = currentBufferView(); + + // Clear non-empty selections + // (e.g. from a "char-forward-select" followed by "char-backward-select") + if (bv) { + Cursor & cur = bv->cursor(); + if ((cur.selection() && cur.selBegin() == cur.selEnd())) { + cur.clearSelection(); + } + } } -- 1.9.1
