Le 20/07/2017 à 11:52, Jürgen Spitzmüller a écrit :
Am Donnerstag, den 20.07.2017, 11:38 +0200 schrieb Jean-Marc
Lasgouttes:
Sigh. It turns out I understood the whole isActive/editable/nargs
issue
only vaguely.
The reason probably is that the naming of these functions is not really
good.
Here is a patch that simplifies the code. The only difference is that
editable() is now true for inner math insets. I am not sure whether it
can affect the Compare feature. The other uses should be OK.
I do not think it should go to master, but rather in 2.4 as an early
cleanup action.
JMarc
From 6c549578b78d768b81f50d354d30c223ff32efb0 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Thu, 20 Jul 2017 12:11:46 +0200
Subject: [PATCH] Cleanup editable and isActive
The only intended change of behavior in this patch is that on-toplevel
math inset will now return editable()==true when isActive() is true.
The situation is now the following:
Semantics:
* isActive: true if a cursor can be placed in this inset
* editable: true if the cursor movement operations will enter this inset.
Definition:
* default: isActive() == (nargs() > 0)
* override: isActive() == false for InsetMathRef and InsetInfo
* defaut: editable() == isActive()
* override: editable() == false for closed InsetCollasapable()
---
src/BufferView.cpp | 1 +
src/Compare.cpp | 2 ++
src/DocIterator.cpp | 5 +----
src/insets/Inset.cpp | 6 ------
src/insets/Inset.h | 6 +++---
src/insets/InsetInfo.h | 2 --
src/insets/InsetTabular.h | 2 --
src/insets/InsetText.h | 2 --
src/mathed/InsetMathHull.h | 2 --
src/mathed/InsetMathMacroTemplate.h | 2 --
src/mathed/InsetMathNest.cpp | 6 ------
src/mathed/InsetMathNest.h | 2 --
12 files changed, 7 insertions(+), 31 deletions(-)
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 2f25636..782f86b 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -792,6 +792,7 @@ bool BufferView::moveToPosition(pit_type bottom_pit, pos_type bottom_pos,
// shortened. Otherwise, setCursor may crash
// lyx when the cursor can not be set to these
// insets.
+ // FIXME: why can't we use the auto_open_ mechanism in this case?
size_t const n = dit.depth();
for (size_t i = 0; i < n; ++i)
if (!dit[i].inset().editable()) {
diff --git a/src/Compare.cpp b/src/Compare.cpp
index 796eb19..684ba21 100644
--- a/src/Compare.cpp
+++ b/src/Compare.cpp
@@ -470,6 +470,7 @@ static bool equal(Inset const * i_o, Inset const * i_n)
// FIXME: This fails if the parameters of the insets differ.
// FIXME: We do not recurse into InsetTabulars.
// FIXME: We need methods inset->equivalent(inset).
+ // FIXME: shouldn't this use isActive() instead?
if (i_o->editable() && !i_o->asInsetMath()
&& i_o->asInsetText())
return true;
@@ -842,6 +843,7 @@ void Compare::Impl::processSnake(DocRangePair const & rp)
DocPair it = rp.from();
for (; it.o < rp.o.to; ++it) {
Inset * inset = it.o.text()->getPar(it.o.pit()).getInset(it.o.pos());
+ // FIXME: consider using isActive() instead
if (inset && inset->editable() && inset->asInsetText()) {
// Find the inset in the paragraph list that will be pasted into
// the final document. The contents of the inset will be replaced
diff --git a/src/DocIterator.cpp b/src/DocIterator.cpp
index 38f2393..6375142 100644
--- a/src/DocIterator.cpp
+++ b/src/DocIterator.cpp
@@ -381,10 +381,7 @@ void DocIterator::forwardPos()
void DocIterator::forwardPosIgnoreCollapsed()
{
Inset * const nextinset = nextInset();
- // FIXME: the check for asInsetMath() shouldn't be necessary
- // but math insets do not return a sensible editable() state yet.
- if (nextinset && !nextinset->asInsetMath()
- && !nextinset->editable()) {
+ if (nextinset && !nextinset->editable()) {
++top().pos();
return;
}
diff --git a/src/insets/Inset.cpp b/src/insets/Inset.cpp
index b458c81..5ea0c2a 100644
--- a/src/insets/Inset.cpp
+++ b/src/insets/Inset.cpp
@@ -484,12 +484,6 @@ bool Inset::directWrite() const
}
-bool Inset::editable() const
-{
- return false;
-}
-
-
bool Inset::hasSettings() const
{
return false;
diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index f10ab3f..758429e 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -348,14 +348,14 @@ public:
virtual bool isActive() const { return nargs() > 0; }
/// can the contents of the inset be edited on screen ?
// equivalent to isActive except for closed InsetCollapsable
- virtual bool editable() const;
- /// has the Inset settings that can be modified in a dialog ?
- virtual bool hasSettings() const;
+ virtual bool editable() const { return isActive(); }
/// can we go further down on mouse click?
/// true for InsetCaption, InsetCollapsables (not ButtonOnly), InsetTabular
virtual bool descendable(BufferView const &) const { return false; }
/// can we click at the specified position ?
virtual bool clickable(BufferView const &, int, int) const { return false; }
+ /// does the Inset have settings that can be modified in a dialog ?
+ virtual bool hasSettings() const;
/// Move one cell backwards
virtual bool allowsCaptionVariation(std::string const &) const { return false; }
// true for insets that have a table structure (InsetMathGrid, InsetTabular)
diff --git a/src/insets/InsetInfo.h b/src/insets/InsetInfo.h
index 221d6fb..d3d6e2c 100644
--- a/src/insets/InsetInfo.h
+++ b/src/insets/InsetInfo.h
@@ -103,8 +103,6 @@ public:
///
bool isActive() const { return false; }
///
- bool editable() const { return false; }
- ///
bool hasSettings() const { return true; }
///
void read(Lexer & lex);
diff --git a/src/insets/InsetTabular.h b/src/insets/InsetTabular.h
index bdd21af..93735c7 100644
--- a/src/insets/InsetTabular.h
+++ b/src/insets/InsetTabular.h
@@ -873,8 +873,6 @@ public:
///
void drawBackground(PainterInfo & pi, int x, int y) const;
///
- bool editable() const { return true; }
- ///
bool hasSettings() const { return true; }
///
bool insetAllowed(InsetCode code) const;
diff --git a/src/insets/InsetText.h b/src/insets/InsetText.h
index eea88cd..3e7caaf 100644
--- a/src/insets/InsetText.h
+++ b/src/insets/InsetText.h
@@ -63,8 +63,6 @@ public:
/// Drawing background is handled in draw
virtual void drawBackground(PainterInfo &, int, int) const {}
///
- bool editable() const { return true; }
- ///
bool canTrackChanges() const { return true; }
/// Rely on RowPainter to draw the cue of inline insets.
bool canPaintChange(BufferView const &) const { return allowMultiPar(); }
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 183b851..cc235a1 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -286,8 +286,6 @@ public:
///
virtual void revealCodes(Cursor & cur) const;
///
- bool editable() const { return true; }
- ///
void edit(Cursor & cur, bool front,
EntryDirection entry_from = ENTRY_DIRECTION_IGNORE);
///
diff --git a/src/mathed/InsetMathMacroTemplate.h b/src/mathed/InsetMathMacroTemplate.h
index ddcfe4d..b450d0c 100644
--- a/src/mathed/InsetMathMacroTemplate.h
+++ b/src/mathed/InsetMathMacroTemplate.h
@@ -37,8 +37,6 @@ public:
/// parses from string, returns false if failed
bool fromString (const docstring & str);
///
- bool editable() const { return true; }
- ///
void edit(Cursor & cur, bool front, EntryDirection entry_from);
///
bool notifyCursorLeaves(Cursor const & old, Cursor & cur);
diff --git a/src/mathed/InsetMathNest.cpp b/src/mathed/InsetMathNest.cpp
index f9c8813..9d2ac69 100644
--- a/src/mathed/InsetMathNest.cpp
+++ b/src/mathed/InsetMathNest.cpp
@@ -312,12 +312,6 @@ void InsetMathNest::lock(bool l)
}
-bool InsetMathNest::isActive() const
-{
- return nargs() > 0;
-}
-
-
MathData InsetMathNest::glue() const
{
MathData ar;
diff --git a/src/mathed/InsetMathNest.h b/src/mathed/InsetMathNest.h
index 3d4977d..3a7e62a 100644
--- a/src/mathed/InsetMathNest.h
+++ b/src/mathed/InsetMathNest.h
@@ -83,8 +83,6 @@ public:
MathData const & cell(idx_type i) const { return cells_[i]; }
//@}
- /// can we move into this cell (see macroarg.h)
- bool isActive() const;
/// request "external features"
void validate(LaTeXFeatures & features) const;
--
2.7.4
