On 10/03/2017 05:28 PM, Richard Heck wrote:
> On 10/03/2017 03:54 PM, Jean-Marc Lasgouttes wrote:
>> Le 03/10/17 à 21:43, Richard Heck a écrit :
>>
>>> The crucial thing here seems to be that it's a "menu" inset. That
>>> triggers a menu search, which calls getStatus, which ends up accessing
>>> the Cursor, which is now out of date. So we end up here:
>> A different solution would be to avoid calling updateInfo() when
>> parsing an info inset. This should be done later in updateBuffer()
>> anyway.
> Yes, I thought of that, too, but I was a bit worried about whether there
> might be other side-effects. Also, the fact that we are, really, only
> doing this once as things are is striking. Maybe do something safe for
> 2.3.x and try something more radical in master?
>
> Looking quickly at this, updateInfo() is also called from setInfo(),
> which is called by the InsetInfo constructor and by doDispatch(), for
> the case of LFUN_INSET_MODIFY. I would think that it should not be
> called there, either.
>
> So maybe something like the attached. I have not tested it. If there's a
> problem, though, adding a forceBufferUpdate() somewhere should fix it.

That didn't work at all. I think the attached should, though.

Richard

>From b224b628bec51e9ca9cf3d96f33f0879b4c4488f Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Tue, 3 Oct 2017 17:28:35 -0400
Subject: [PATCH] Remove updateInfo() calls in favor of doing the relevant work
 in updateBuffer().

Also adds a flag to tell us whether we need to recalculate the
relevant information. In some cases, there is no need to do so
more than once. (Note that, with the existing code, we *never*
recalculate, which would have given rise to bugs.)
---
 src/factory.cpp          |  1 -
 src/insets/InsetInfo.cpp | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 src/insets/InsetInfo.h   |  6 ++++--
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/src/factory.cpp b/src/factory.cpp
index 72019cd..3937a9e 100644
--- a/src/factory.cpp
+++ b/src/factory.cpp
@@ -263,7 +263,6 @@ Inset * createInsetHelper(Buffer * buf, FuncRequest const & cmd)
 
 		case LFUN_INFO_INSERT: {
 			InsetInfo * inset = new InsetInfo(buf, to_utf8(cmd.argument()));
-			inset->updateInfo();
 			return inset;
 		}
 
diff --git a/src/insets/InsetInfo.cpp b/src/insets/InsetInfo.cpp
index 42e0f6c..d52f887 100644
--- a/src/insets/InsetInfo.cpp
+++ b/src/insets/InsetInfo.cpp
@@ -92,7 +92,8 @@ NameTranslator const & nameTranslator()
 
 
 InsetInfo::InsetInfo(Buffer * buf, string const & name)
-	: InsetCollapsable(buf), type_(UNKNOWN_INFO), name_()
+	: InsetCollapsable(buf), initialized_(false), 
+	  type_(UNKNOWN_INFO), name_()
 {
 	setInfo(name);
 	status_ = Collapsed;
@@ -147,7 +148,6 @@ void InsetInfo::read(Lexer & lex)
 			_("Missing \\end_inset at this point."),
 			from_utf8(token));
 	}
-	updateInfo();
 }
 
 
@@ -247,6 +247,7 @@ void InsetInfo::doDispatch(Cursor & cur, FuncRequest & cmd)
 	case LFUN_INSET_MODIFY:
 		cur.recordUndo();
 		setInfo(to_utf8(cmd.argument()));
+		initialized_ = false;
 		break;
 
 	case LFUN_INSET_COPY_AS: {
@@ -278,7 +279,6 @@ void InsetInfo::setInfo(string const & name)
 	string type;
 	name_ = trim(split(name, type, ' '));
 	type_ = nameTranslator().find(type);
-	updateInfo();
 }
 
 
@@ -295,16 +295,21 @@ void InsetInfo::setText(docstring const & str)
 }
 
 
-void InsetInfo::updateInfo()
-{
+void InsetInfo::updateBuffer(ParIterator const & it, UpdateType utype) {
 	BufferParams const & bp = buffer().params();
 
 	switch (type_) {
 	case UNKNOWN_INFO:
 		error("Unknown Info: %1$s");
+		initialized_ = false;
 		break;
 	case SHORTCUT_INFO:
 	case SHORTCUTS_INFO: {
+		// only need to do this once.
+		if (initialized_)
+			break;
+		// and we will not keep trying if we fail
+		initialized_ = true;
 		FuncRequest const func = lyxaction.lookupFunc(name_);
 		if (func.action() == LFUN_UNKNOWN_ACTION) {
 			error("Unknown action %1$s");
@@ -323,11 +328,15 @@ void InsetInfo::updateInfo()
 		break;
 	}
 	case LYXRC_INFO: {
+		// this information could actually change, if the preferences are changed,
+		// so we will recalculate each time through.
 		ostringstream oss;
 		if (name_.empty()) {
 			setText(_("undefined"));
 			break;
 		}
+		// FIXME this uses the serialization mechanism to get the info
+		// we want, which i guess works but is a bit strange.
 		lyxrc.write(oss, true, name_);
 		string result = oss.str();
 		if (result.size() < 2) {
@@ -351,20 +360,33 @@ void InsetInfo::updateInfo()
 		break;
 	}
 	case PACKAGE_INFO:
+		// only need to do this once.
+		if (initialized_)
+			break;
 		// check in packages.lst
 		setText(LaTeXFeatures::isAvailable(name_) ? _("yes") : _("no"));
+		initialized_ = true;
 		break;
 
 	case TEXTCLASS_INFO: {
+		// only need to do this once.
+		if (initialized_)
+			break;
 		// name_ is the class name
 		LayoutFileList const & list = LayoutFileList::get();
 		bool available = false;
 		if (list.haveClass(name_))
 			available = list[name_].isTeXClassAvailable();
 		setText(available ? _("yes") : _("no"));
+		initialized_ = true;
 		break;
 	}
 	case MENU_INFO: {
+		// only need to do this once.
+		if (initialized_)
+			break;
+		// and we will not keep trying if we fail
+		initialized_ = true;
 		docstring_list names;
 		FuncRequest const func = lyxaction.lookupFunc(name_);
 		if (func.action() == LFUN_UNKNOWN_ACTION) {
@@ -406,6 +428,11 @@ void InsetInfo::updateInfo()
 		break;
 	}
 	case ICON_INFO: {
+		// only need to do this once.
+		if (initialized_)
+			break;
+		// and we will not keep trying if we fail
+		initialized_ = true;
 		FuncRequest func = lyxaction.lookupFunc(name_);
 		docstring icon_name = frontend::Application::iconName(func, true);
 		// FIXME: We should use the icon directly instead of
@@ -451,6 +478,7 @@ void InsetInfo::updateInfo()
 		break;
 	}
 	case BUFFER_INFO: {
+		// this could all change, so we will recalculate each time
 		if (name_ == "name") {
 			setText(from_utf8(buffer().fileName().onlyFileName()));
 			break;
@@ -464,8 +492,12 @@ void InsetInfo::updateInfo()
 			break;
 		}
 
+		////////////////////////////////////////////////////////////////
 		// everything that follows is for version control.
 		// nothing that isn't version control should go below this line.
+		
+		// this information could change, in principle, so we will 
+		// recalculate each time through
 		if (!buffer().lyxvc().inUse()) {
 			setText(_("No version control"));
 			break;
@@ -489,10 +521,15 @@ void InsetInfo::updateInfo()
 		break;
 	}
 	case LYX_INFO:
+		// only need to do this once.
+		if (initialized_)
+			break;
 		if (name_ == "version")
 			setText(from_ascii(lyx_version));
+		initialized_ = true;
 		break;
 	}
+	InsetCollapsable::updateBuffer(it, utype);
 }
 
 
diff --git a/src/insets/InsetInfo.h b/src/insets/InsetInfo.h
index 96ba22c..8ab6f82 100644
--- a/src/insets/InsetInfo.h
+++ b/src/insets/InsetInfo.h
@@ -127,8 +127,8 @@ public:
 	void doDispatch(Cursor & cur, FuncRequest & cmd);
 	///
 	void setInfo(std::string const & info);
-	/// update info_ and text
-	void updateInfo();
+	///
+	void updateBuffer(ParIterator const & it, UpdateType utype);
 	///
 	docstring toolTip(BufferView const & bv, int x, int y) const;
 	///
@@ -148,6 +148,8 @@ private:
 	// make sure that the other version of setText is still available.
 	using InsetCollapsable::setText;
 	///
+	bool initialized_;
+	///
 	info_type type_;
 	///
 	std::string name_;
-- 
2.9.5

Reply via email to