[LyX/master] Revert "Improve structure of updateMacros"

[Jean-Marc Lasgouttes]

Le 11/11/2020 à 17:13, Richard Kimberly Heck a écrit :
> commit fdd0a6f84923f1d4a1f82457a3e256a2ee706f6b
> Author: Richard Kimberly Heck <rikiheck at lyx.org>
> Date:   Wed Nov 11 11:24:59 2020 -0500
> 
>      Revert "Improve structure of updateMacros"
>      
>      The problem here is that the check against TEXT_CODE only
>      catches InsetText, and not its subclasses. Same for the check
>      against MATH_HULL_CODE. So there is reason not to check codes
>      here.

I really think that we should reduce our use of codes as much as 
possible. Isn't it possible to use a virtual method here that would be 
implemented by each inset? t is bad to have the knowledge about the 
insets scattered around the code.

JMarc

>      
>      This reverts commit 5a54ccfa87057fd3220d7193b40ac2dd37a9e6e1.
> ---
>   src/Buffer.cpp          |   79 +++++++++++++++++++++++------------------------
>   src/mathed/MathData.cpp |    4 +-
>   2 files changed, 41 insertions(+), 42 deletions(-)
> 
> diff --git a/src/Buffer.cpp b/src/Buffer.cpp
> index acda58f..23f6315 100644
> --- a/src/Buffer.cpp
> +++ b/src/Buffer.cpp
> @@ -3730,11 +3730,10 @@ 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?
> -			case TEXT_CODE: {
> -				InsetText const * itext = insit.inset->asInsetText();
> +			if (insit.inset->asInsetText()) {
>   				// Inset needs its own scope?
> +				InsetText const * itext = insit.inset->asInsetText();
>   				bool newScope = itext->isMacroScope();
>   
>   				// scope which ends just behind the inset
> @@ -3745,9 +3744,10 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
>   				it.push_back(CursorSlice(*insit.inset));
>   				updateMacros(it, newScope ? insetScope : scope);
>   				it.pop_back();
> -				break;
> +				continue;
>   			}
> -			case TABULAR_CODE: {
> +
> +			if (insit.inset->asInsetTabular()) {
>   				CursorSlice slice(*insit.inset);
>   				size_t const numcells = slice.nargs();
>   				for (; slice.idx() < numcells; slice.forwardIdx()) {
> @@ -3755,10 +3755,11 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
>   					updateMacros(it, scope);
>   					it.pop_back();
>   				}
> -				break;
> +				continue;
>   			}
> +
>   			// is it an external file?
> -			case INCLUDE_CODE: {
> +			if (insit.inset->lyxCode() == INCLUDE_CODE) {
>   				// get buffer of external file
>   				InsetInclude const & incinset =
>   					static_cast<InsetInclude const &>(*insit.inset);
> @@ -3766,7 +3767,7 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
>   				Buffer * child = incinset.loadIfNeeded();
>   				macro_lock = false;
>   				if (!child)
> -					break;
> +					continue;
>   
>   				// register its position, but only when it is
>   				// included first in the buffer
> @@ -3774,40 +3775,38 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
>   
>   				// register child with its scope
>   				position_to_children[it] = Impl::ScopeBuffer(scope, child);
> -				break;
> -			}
> -			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(owner_, it));
> -				break;
> +				continue;
>   			}
> -			default:
> -				break;
> +
> +			InsetMath * im = insit.inset->asInsetMath();
> +			if (doing_export && im)  {
> +				InsetMathHull * hull = im->asHullInset();
> +				if (hull)
> +					hull->recordLocation(it);
>   			}
> +
> +			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 ce1f4b5..39d5f80 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() : nullptr;
> -	InsetMathMacro const * inmacro = inmath ? inmath->asMacro() : nullptr;
> +	InsetMath const * inmath = cur ? cur->inset().asInsetMath() : 0;
> +	InsetMathMacro const * inmacro = inmath ? inmath->asMacro() : 0;
>   	docstring const edited_name = inmacro ? inmacro->name() : docstring();
>   
>   	// go over the array and look for macros
> 
devel