[LyX features/cleanup/updateMacros] The isMacroScope method is just !producesOutput (or should be).

Richard Kimberly Heck rikiheck at lyx.org
Sun Nov 8 07:32:16 UTC 2020


The branch, cleanup/updateMacros, has been updated.
  discards  9058af5721689eb5028507195ccdb1e59ff226f9 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (9058af5721689eb5028507195ccdb1e59ff226f9)
            \
             N -- N -- N (1c3a66b47778412f600d18ace51375482dd94df9)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

- Log -----------------------------------------------------------------

commit 1c3a66b47778412f600d18ace51375482dd94df9
Author: Richard Kimberly Heck <rikiheck at lyx.org>
Date:   Fri Nov 6 11:41:20 2020 -0500

    The isMacroScope method is just !producesOutput (or should be).
    
    So remove that, and restructure the code a bit.

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index acda58f..44b3178 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -3734,16 +3734,20 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 			// is it a nested text inset?
 			case TEXT_CODE: {
 				InsetText const * itext = insit.inset->asInsetText();
-				// Inset needs its own scope?
-				bool newScope = itext->isMacroScope();
-
-				// scope which ends just behind the inset
-				DocIterator insetScope = it;
-				++insetScope.pos();
 
 				// collect macros in inset
 				it.push_back(CursorSlice(*insit.inset));
-				updateMacros(it, newScope ? insetScope : scope);
+				if (itext->producesOutput()) {
+					// the simple case
+					updateMacros(it, scope);
+				} else {
+					// We don't want macros declared in notes, comments, etc, 
+					// to affect anything outside them.
+					// New scope which ends just behind the inset
+					DocIterator new_scope = it;
+					++new_scope.pos();
+					updateMacros(it, new_scope);
+				}
 				it.pop_back();
 				break;
 			}
diff --git a/src/insets/InsetBranch.cpp b/src/insets/InsetBranch.cpp
index a70ff27..17e8007 100644
--- a/src/insets/InsetBranch.cpp
+++ b/src/insets/InsetBranch.cpp
@@ -381,13 +381,6 @@ string InsetBranch::contextMenuName() const
 }
 
 
-bool InsetBranch::isMacroScope() const
-{
-	// Its own scope if not selected by buffer
-	return !producesOutput();
-}
-
-
 string InsetBranch::params2string(InsetBranchParams const & params)
 {
 	ostringstream data;
diff --git a/src/insets/InsetBranch.h b/src/insets/InsetBranch.h
index 6106ce5..2746485 100644
--- a/src/insets/InsetBranch.h
+++ b/src/insets/InsetBranch.h
@@ -104,8 +104,6 @@ private:
 	///
 	bool getStatus(Cursor &, FuncRequest const &, FuncStatus &) const override;
 	///
-	bool isMacroScope() const override;
-	///
 	docstring toolTip(BufferView const & bv, int x, int y) const override;
 	///
 	bool usePlainLayout() const override { return false; }
diff --git a/src/insets/InsetNote.cpp b/src/insets/InsetNote.cpp
index fa84c61..5717feb 100644
--- a/src/insets/InsetNote.cpp
+++ b/src/insets/InsetNote.cpp
@@ -198,16 +198,6 @@ bool InsetNote::getStatus(Cursor & cur, FuncRequest const & cmd,
 }
 
 
-bool InsetNote::isMacroScope() const
-{
-	// LyX note has no latex output
-	if (params_.type == InsetNoteParams::Note)
-		return true;
-
-	return InsetCollapsible::isMacroScope();
-}
-
-
 void InsetNote::latex(otexstream & os, OutputParams const & runparams_in) const
 {
 	if (params_.type == InsetNoteParams::Note)
diff --git a/src/insets/InsetNote.h b/src/insets/InsetNote.h
index aeab8f0..b698505 100644
--- a/src/insets/InsetNote.h
+++ b/src/insets/InsetNote.h
@@ -79,8 +79,6 @@ private:
 	/// show the note dialog
 	bool showInsetDialog(BufferView * bv) const override;
 	///
-	bool isMacroScope() const override;
-	///
 	void latex(otexstream &, OutputParams const &) const override;
 	///
 	int plaintext(odocstringstream & ods, OutputParams const & op,
diff --git a/src/insets/InsetText.h b/src/insets/InsetText.h
index d571806..cefd44c 100644
--- a/src/insets/InsetText.h
+++ b/src/insets/InsetText.h
@@ -156,8 +156,6 @@ public:
 	///
 	bool allowSpellCheck() const override { return getLayout().spellcheck() && !getLayout().isPassThru(); }
 	///
-	virtual bool isMacroScope() const { return false; }
-	///
 	bool allowMultiPar() const override { return getLayout().isMultiPar(); }
 	///
 	bool isInTitle() const override { return intitle_context_; }

commit 589e780e153e8cddd577b31316248e1c457c29e3
Author: Yuriy Skalko <yuriy.skalko at gmail.com>
Date:   Thu Nov 5 12:01:21 2020 +0200

    Whitespace & renaming

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 4e704e3..bb41759 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -504,9 +504,9 @@ void BufferView::processUpdateFlags(Update::flags flags)
 	// We handle this before FitCursor because the later will require
 	// correct metrics at cursor position.
 	if (!(flags & Update::ForceDraw)
-	    && (flags & Update::SinglePar)
-		&& !singleParUpdate())
-			updateMetrics(flags);
+			&& (flags & Update::SinglePar)
+			&& !singleParUpdate())
+		updateMetrics(flags);
 
 	// Then make sure that the screen contains the cursor if needed
 	if (flags & Update::FitCursor) {
diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index bf165d9..c6fbfdb 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -529,8 +529,8 @@ public:
 	///
 	virtual bool allowSpellCheck() const { return false; }
 
-	/// if this insets owns text cells (e.g. InsetText) return cell num
-	virtual Text * getText(int /*num*/) const { return 0; }
+	/// if this insets owns text cells (e.g. InsetText) return cell idx
+	virtual Text * getText(int /*idx*/) const { return 0; }
 
 	/** Adds a LaTeX snippet to the Preview Loader for transformation
 	 *  into a bitmap image. Does not start the loading process.
diff --git a/src/insets/InsetText.h b/src/insets/InsetText.h
index bccaacc..d571806 100644
--- a/src/insets/InsetText.h
+++ b/src/insets/InsetText.h
@@ -114,8 +114,8 @@ public:
 	///
 	void setFrameColor(ColorCode);
 	///
-	Text * getText(int i) const override {
-		return (i == 0) ? const_cast<Text*>(&text_) : 0;
+	Text * getText(int idx) const override {
+		return (idx == 0) ? const_cast<Text*>(&text_) : nullptr;
 	}
 	///
 	bool getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus &) const override;

commit 01e673635b0701cd0cd19ef96bc5e4b3879b6db1
Author: Yuriy Skalko <yuriy.skalko at gmail.com>
Date:   Wed Nov 4 12:04:39 2020 +0200

    MacroData refactoring

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index e86450d..acda58f 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -3802,7 +3802,7 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 				// FIXME (Abdel), I don't understand why we pass 'it' here
 				// instead of 'macroTemplate' defined above... is this correct?
 				macros[macroTemplate.name()][it] =
-					Impl::ScopeMacro(scope, MacroData(const_cast<Buffer *>(owner_), it));
+					Impl::ScopeMacro(scope, MacroData(owner_, it));
 				break;
 			}
 			default:
diff --git a/src/mathed/MacroTable.cpp b/src/mathed/MacroTable.cpp
index fb2a9e0..218c849 100644
--- a/src/mathed/MacroTable.cpp
+++ b/src/mathed/MacroTable.cpp
@@ -41,23 +41,18 @@ namespace lyx {
 //
 /////////////////////////////////////////////////////////////////////
 
-MacroData::MacroData(Buffer * buf)
-	: buffer_(buf), queried_(true), numargs_(0), sym_(0), optionals_(0),
-	  lockCount_(0), redefinition_(false), type_(MacroTypeNewcommand)
+MacroData::MacroData(const Buffer * buf)
+	: buffer_(buf), queried_(true)
 {}
 
 
-MacroData::MacroData(Buffer * buf, DocIterator const & pos)
-	: buffer_(buf), pos_(pos), queried_(false), numargs_(0), sym_(0),
-	  optionals_(0), lockCount_(0), redefinition_(false),
-	  type_(MacroTypeNewcommand)
-{
-}
+MacroData::MacroData(Buffer const * buf, DocIterator const & pos)
+	: buffer_(buf), pos_(pos)
+{}
 
 
-MacroData::MacroData(Buffer * buf, InsetMathMacroTemplate const & macro)
-	: buffer_(buf), queried_(false), numargs_(0), sym_(0), optionals_(0),
-	  lockCount_(0), redefinition_(false), type_(MacroTypeNewcommand)
+MacroData::MacroData(Buffer const * buf, InsetMathMacroTemplate const & macro)
+	: buffer_(buf)
 {
 	queryData(macro);
 }
diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index b588ee2..77b306f 100644
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -38,11 +38,11 @@ enum MacroType {
 class MacroData {
 public:
 	/// Constructor to make STL containers happy
-	explicit MacroData(Buffer * buf = 0);
+	explicit MacroData(Buffer const * buf = 0);
 	/// Create lazy MacroData which only queries the macro template when needed
-	MacroData(Buffer * buf, DocIterator const & pos);
+	MacroData(Buffer const * buf, DocIterator const & pos);
 	/// Create non-lazy MacroData which directly queries the macro template
-	MacroData(Buffer * buf, InsetMathMacroTemplate const & macro);
+	MacroData(Buffer const * buf, InsetMathMacroTemplate const & macro);
 
 	///
 	docstring const & definition() const { updateData(); return definition_; }
@@ -122,25 +122,25 @@ private:
 	/// macros.
 	mutable DocIterator pos_;
 	///
-	mutable bool queried_;
+	mutable bool queried_ = false;
 	///
 	mutable docstring definition_;
 	///
-	mutable size_t numargs_;
+	mutable size_t numargs_ = 0;
 	///
 	mutable docstring display_;
 	///
-	latexkeys const * sym_;
+	latexkeys const * sym_ = nullptr;
 	///
-	mutable size_t optionals_;
+	mutable size_t optionals_ = 0;
 	///
 	mutable std::vector<docstring> defaults_;
 	///
-	mutable int lockCount_;
+	mutable int lockCount_ = 0;
 	///
-	mutable bool redefinition_;
+	mutable bool redefinition_ = false;
 	///
-	mutable MacroType type_;
+	mutable MacroType type_ = MacroTypeNewcommand;
 };
 
 

commit 5a54ccfa87057fd3220d7193b40ac2dd37a9e6e1
Author: Yuriy Skalko <yuriy.skalko at gmail.com>
Date:   Wed Nov 4 11:27:08 2020 +0200

    Improve structure of updateMacros

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 23f6315..e86450d 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -3730,10 +3730,11 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 		for (auto const & insit : par.insetList()) {
 			it.pos() = insit.pos;
 
+			switch (insit.inset->lyxCode()) {
 			// is it a nested text inset?
-			if (insit.inset->asInsetText()) {
-				// Inset needs its own scope?
+			case TEXT_CODE: {
 				InsetText const * itext = insit.inset->asInsetText();
+				// Inset needs its own scope?
 				bool newScope = itext->isMacroScope();
 
 				// scope which ends just behind the inset
@@ -3744,10 +3745,9 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 				it.push_back(CursorSlice(*insit.inset));
 				updateMacros(it, newScope ? insetScope : scope);
 				it.pop_back();
-				continue;
+				break;
 			}
-
-			if (insit.inset->asInsetTabular()) {
+			case TABULAR_CODE: {
 				CursorSlice slice(*insit.inset);
 				size_t const numcells = slice.nargs();
 				for (; slice.idx() < numcells; slice.forwardIdx()) {
@@ -3755,11 +3755,10 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 					updateMacros(it, scope);
 					it.pop_back();
 				}
-				continue;
+				break;
 			}
-
 			// is it an external file?
-			if (insit.inset->lyxCode() == INCLUDE_CODE) {
+			case INCLUDE_CODE: {
 				// get buffer of external file
 				InsetInclude const & incinset =
 					static_cast<InsetInclude const &>(*insit.inset);
@@ -3767,7 +3766,7 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 				Buffer * child = incinset.loadIfNeeded();
 				macro_lock = false;
 				if (!child)
-					continue;
+					break;
 
 				// register its position, but only when it is
 				// included first in the buffer
@@ -3775,38 +3774,40 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
 
 				// register child with its scope
 				position_to_children[it] = Impl::ScopeBuffer(scope, child);
-				continue;
+				break;
 			}
-
-			InsetMath * im = insit.inset->asInsetMath();
-			if (doing_export && im)  {
-				InsetMathHull * hull = im->asHullInset();
-				if (hull)
-					hull->recordLocation(it);
+			case MATH_HULL_CODE: {
+				if (!doing_export)
+					break;
+				InsetMathHull * hull = insit.inset->asInsetMath()->asHullInset();
+				hull->recordLocation(it);
+				break;
+			}
+			case MATHMACRO_CODE: {
+				// get macro data
+				InsetMathMacroTemplate & macroTemplate =
+					*insit.inset->asInsetMath()->asMacroTemplate();
+				MacroContext mc(owner_, it);
+				macroTemplate.updateToContext(mc);
+	
+				// valid?
+				bool valid = macroTemplate.validMacro();
+				// FIXME: Should be fixNameAndCheckIfValid() in fact,
+				// then the BufferView's cursor will be invalid in
+				// some cases which leads to crashes.
+				if (!valid)
+					break;
+	
+				// register macro
+				// FIXME (Abdel), I don't understand why we pass 'it' here
+				// instead of 'macroTemplate' defined above... is this correct?
+				macros[macroTemplate.name()][it] =
+					Impl::ScopeMacro(scope, MacroData(const_cast<Buffer *>(owner_), it));
+				break;
+			}
+			default:
+				break;
 			}
-
-			if (insit.inset->lyxCode() != MATHMACRO_CODE)
-				continue;
-
-			// get macro data
-			InsetMathMacroTemplate & macroTemplate =
-				*insit.inset->asInsetMath()->asMacroTemplate();
-			MacroContext mc(owner_, it);
-			macroTemplate.updateToContext(mc);
-
-			// valid?
-			bool valid = macroTemplate.validMacro();
-			// FIXME: Should be fixNameAndCheckIfValid() in fact,
-			// then the BufferView's cursor will be invalid in
-			// some cases which leads to crashes.
-			if (!valid)
-				continue;
-
-			// register macro
-			// FIXME (Abdel), I don't understand why we pass 'it' here
-			// instead of 'macroTemplate' defined above... is this correct?
-			macros[macroTemplate.name()][it] =
-				Impl::ScopeMacro(scope, MacroData(const_cast<Buffer *>(owner_), it));
 		}
 
 		// next paragraph
diff --git a/src/mathed/MathData.cpp b/src/mathed/MathData.cpp
index 39d5f80..ce1f4b5 100644
--- a/src/mathed/MathData.cpp
+++ b/src/mathed/MathData.cpp
@@ -403,8 +403,8 @@ void MathData::updateMacros(Cursor * cur, MacroContext const & mc,
 {
 	// If we are editing a macro, we cannot update it immediately,
 	// otherwise wrong undo steps will be recorded (bug 6208).
-	InsetMath const * inmath = cur ? cur->inset().asInsetMath() : 0;
-	InsetMathMacro const * inmacro = inmath ? inmath->asMacro() : 0;
+	InsetMath const * inmath = cur ? cur->inset().asInsetMath() : nullptr;
+	InsetMathMacro const * inmacro = inmath ? inmath->asMacro() : nullptr;
 	docstring const edited_name = inmacro ? inmacro->name() : docstring();
 
 	// go over the array and look for macros

-----------------------------------------------------------------------

Summary of changes:
 src/Buffer.cpp            |   79 +++++++++++++++++++++++---------------------
 src/BufferView.cpp        |    6 ++--
 src/insets/Inset.h        |    4 +-
 src/insets/InsetText.h    |    4 +-
 src/mathed/MacroTable.cpp |   19 ++++-------
 src/mathed/MacroTable.h   |   20 ++++++------
 src/mathed/MathData.cpp   |    4 +-
 7 files changed, 67 insertions(+), 69 deletions(-)


hooks/post-receive
-- 
Repository for new features


More information about the lyx-cvs mailing list