Le 18/09/2015 16:06, Cyrille Artho a écrit :
Jürgen Spitzmüller wrote:
2015-09-18 2:37 GMT+02:00 Guillaume Munch <g...@lyx.org
<mailto:g...@lyx.org>>:


        And in fact, I'd also like to navigate there directly.  > >
        Navigation would be analoguous to sections that have subsections,

    so >  this seems just right to me. > >> Do you have a situation in
    mind where having the 3rd-level menu >> would gain substantially
    more time than just clicking the 2nd-level >> element? > > Time is
    not everything. I think the extra time investment is not too >  big
    a burden.
    Regarding the navigation menu, the possibility of going through only
    2 menus instead of 3 makes a big difference in UI and time is only
    one aspect of the increased convenience. I'd take it as soon as
    there's the possibility. My point about which you seem to disagree
    is that there's such a possibility, because a subfloat is different
    from a subsection in that a subfloat is always close to the
    beginning of its float. Not showing the subfloats in the navigation
    menu is a deliberate choice that seems smart to me given the context.

    Since the difference appears to be a matter of taste, has anybody an
    opinion on this?


My opinion is that the navigation menu should display the same structure
than the outliner.

I think it is a good idea to put subfloats one level deeper in the
menus. It reduces clutter while keeping the subfloats available (for
example, you may remember the title of a subfloat but not of its
enclosing float, so you can still find it by going through each float).



The attached patch includes the above features along with other enhancements like a symbol that indicates disabled entries. (See description.) OK to commit?
>From 63a3c877c398976fc133368f157274ebeba724eb Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
Date: Sun, 27 Sep 2015 07:05:00 +0100
Subject: [PATCH] Enhancements and bugfixes to the TOCs

* Lists of floats now show subfloats deeper in the navigation menu

* The arbitrary 30 element cut-off after which nothing is shown except "Open
  Navigator..." is removed. Menus now have no limit in size, so Qt may display
  them scrollable. In exchange, we always show "Open outliner..." at the
  beginning. I tested for performance issues with a rather complex document and
  it is fine; but this does not exclude corner cases with lots of TOC entries of
  a certain kind. If necessary, populating the navigation sub-menu should be
  delayed like the main menu.

* Elements that do not contribute to the output (e.g. in a note, a disabled
  branch) are now preceded with a symbol indicating this status. (The machinery
  was already there; I wonder why it was not implemented already.) I have chosen
  U+274E NEGATIVE SQUARED CROSS MARK.

* Fix the contextual menus in the outliner (bug introduced at 94e992c5).

* Toc item now move to the caption when present, but first center on the float,
  to prevent the situation where the caption is at the top of the screen and the
  contents of the float is off-screen above the caption.
  (Internally, the action of the toc items can now be customised)
---
 src/Buffer.cpp                  |  2 +-
 src/TocBackend.cpp              | 51 ++++++++++++++++++++++-----
 src/TocBackend.h                | 17 ++++++---
 src/frontends/qt4/Menus.cpp     | 77 ++++++++++++-----------------------------
 src/frontends/qt4/TocModel.cpp  | 10 +++---
 src/frontends/qt4/TocWidget.cpp | 18 ++++------
 6 files changed, 90 insertions(+), 85 deletions(-)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 0f37edd..1af333c 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -4462,7 +4462,7 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
 	d->bibinfo_cache_valid_ = true;
 	d->cite_labels_valid_ = true;
 	/// FIXME: Perf
-	cbuf.tocBackend().update(utype == OutputUpdate);
+	cbuf.tocBackend().update(true);
 	if (scope == UpdateMaster)
 		cbuf.structureChanged();
 }
diff --git a/src/TocBackend.cpp b/src/TocBackend.cpp
index 1eb8f80..04f1168 100644
--- a/src/TocBackend.cpp
+++ b/src/TocBackend.cpp
@@ -39,6 +39,14 @@ using namespace std;
 
 namespace lyx {
 
+// convert a DocIterator into an argument to LFUN_PARAGRAPH_GOTO 
+docstring paragraph_goto_arg(DocIterator const & dit)
+{
+	return convert<docstring>(dit.paragraph().id()) + ' ' +
+		convert<docstring>(dit.pos());
+}
+
+
 ///////////////////////////////////////////////////////////////////////////
 //
 // TocItem implementation
@@ -46,8 +54,9 @@ namespace lyx {
 ///////////////////////////////////////////////////////////////////////////
 
 TocItem::TocItem(DocIterator const & dit, int d, docstring const & s,
-	bool output_active, docstring const & t) :
-  dit_(dit), depth_(d), str_(s), tooltip_(t), output_(output_active)
+			bool output_active, docstring const & t, FuncRequest action) :
+	dit_(dit), depth_(d), str_(s), tooltip_(t), output_(output_active),
+	action_(action)
 {
 }
 
@@ -66,15 +75,25 @@ docstring const & TocItem::tooltip() const
 
 docstring const TocItem::asString() const
 {
-	return docstring(4 * depth_, ' ') + str_;
+	// U+2327 X IN A RECTANGLE BOX
+	// char_type const cross = 0x2327;
+	// U+274E NEGATIVE SQUARED CROSS MARK
+	char_type const cross = 0x274e;
+	docstring prefix;
+	if (!output_) {
+		prefix += cross;
+		prefix += " ";
+	}
+	return prefix + str_;
 }
 
 
 FuncRequest TocItem::action() const
 {
-	string const arg = convert<string>(dit_.paragraph().id())
-		+ ' ' + convert<string>(dit_.pos());
-	return FuncRequest(LFUN_PARAGRAPH_GOTO, arg);
+	if (action_.action() == LFUN_UNKNOWN_ACTION) {
+		return FuncRequest(LFUN_PARAGRAPH_GOTO, paragraph_goto_arg(dit_));
+	} else
+		return action_;
 }
 
 
@@ -156,15 +175,29 @@ void TocBuilder::pushItem(DocIterator const & dit, docstring const & s,
 void TocBuilder::captionItem(DocIterator const & dit, docstring const & s,
 							 bool output_active)
 {
+	// first show the float before moving to the caption
+	docstring arg = "paragraph-goto " + paragraph_goto_arg(dit);
+	if (!stack_.empty())
+		arg = "paragraph-goto " +
+			paragraph_goto_arg((*toc_)[stack_.top().pos].dit_) + ";" + arg;
+	FuncRequest func(LFUN_COMMAND_SEQUENCE, arg);
+	
 	if (!stack_.empty() && !stack_.top().is_captioned) {
 		// The float we entered has not yet been assigned a caption.
 		// Assign the caption string to it.
-		(*toc_)[stack_.top().pos].str(s);
+		TocItem & captionable = (*toc_)[stack_.top().pos];
+		captionable.str(s);
+		captionable.setAction(func);
 		stack_.top().is_captioned = true;
 	} else {
 		// This is a new entry.
 		pop();
-		pushItem(dit, s, output_active, true);
+		// the dit is at the float's level, e.g. for the contextual menu of
+		// outliner entries
+		DocIterator captionable_dit = dit;
+		captionable_dit.pop_back();
+		pushItem(captionable_dit, s, output_active, true);
+		(*toc_)[stack_.top().pos].setAction(func);
 	}
 }
 
@@ -269,7 +302,7 @@ bool TocBackend::updateItem(DocIterator const & dit)
 		&& tocstring.empty())
 			tocstring = par.asString(AS_STR_LABEL | AS_STR_INSETS);
 
-	const_cast<TocItem &>(*toc_item).str_ = tocstring;
+	const_cast<TocItem &>(*toc_item).str(tocstring);
 
 	buffer_->updateTocItem("tableofcontents", dit);
 	return true;
diff --git a/src/TocBackend.h b/src/TocBackend.h
index 176175d..a34d515 100644
--- a/src/TocBackend.h
+++ b/src/TocBackend.h
@@ -16,6 +16,7 @@
 #define TOC_BACKEND_H
 
 #include "DocIterator.h"
+#include "FuncRequest.h"
 
 #include "support/shared_ptr.h"
 #include "support/strfwd.h"
@@ -29,7 +30,6 @@
 namespace lyx {
 
 class Buffer;
-class FuncRequest;
 
 
 /* FIXME: toc types are currently identified by strings. It cannot be converted
@@ -66,6 +66,7 @@ class TocItem
 {
 	friend class Toc;
 	friend class TocBackend;
+	friend class TocBuilder;
 
 public:
 	/// Default constructor for STL containers.
@@ -75,7 +76,8 @@ public:
 		int depth,
 		docstring const & s,
 		bool output_active,
-		docstring const & t = docstring()
+		docstring const & t = docstring(),
+		FuncRequest action = FuncRequest(LFUN_UNKNOWN_ACTION)
 		);
 	///
 	~TocItem() {}
@@ -89,19 +91,22 @@ public:
 	void str(docstring const & s) { str_ = s; }
 	///
 	docstring const & tooltip() const;
-	///
+	/// String for display, e.g. it has a mark if output is inactive
 	docstring const asString() const;
 	///
 	DocIterator const & dit() const { return dit_; }
 	///
 	bool isOutput() const { return output_; }
-
-	/// the action corresponding to the goTo above
+	///
+	void setAction(FuncRequest a) { action_ = a; }
+	/// custom action, or the default one (paragraph-goto) if not customised
 	FuncRequest action() const;
 
 protected:
 	/// Current position of item.
 	DocIterator dit_;
+
+private:
 	/// nesting depth
 	int depth_;
 	/// Full item string
@@ -110,6 +115,8 @@ protected:
 	docstring tooltip_;
 	/// Is this item in a note, inactive branch, etc?
 	bool output_;
+	/// Custom action
+	FuncRequest action_;
 };
 
 
diff --git a/src/frontends/qt4/Menus.cpp b/src/frontends/qt4/Menus.cpp
index c336542..592a66a 100644
--- a/src/frontends/qt4/Menus.cpp
+++ b/src/frontends/qt4/Menus.cpp
@@ -1219,7 +1219,7 @@ void MenuDefinition::expandFlexInsert(
 }
 
 
-size_t const max_number_of_items = 25;
+size_t const max_number_of_items = 30;
 
 void MenuDefinition::expandToc2(Toc const & toc_list,
 		size_t from, size_t to, int depth)
@@ -1236,7 +1236,7 @@ void MenuDefinition::expandToc2(Toc const & toc_list,
 	if (to - from <= max_number_of_items) {
 		for (size_t i = from; i < to; ++i) {
 			QString label(4 * max(0, toc_list[i].depth() - depth), ' ');
-			label += limitStringLength(toc_list[i].str());
+			label += limitStringLength(toc_list[i].asString());
 			if (toc_list[i].depth() == depth) {
 				label += '|';
 			    if (shortcut_count < 9) {
@@ -1246,6 +1246,9 @@ void MenuDefinition::expandToc2(Toc const & toc_list,
 			}
 			add(MenuItem(MenuItem::Command, label,
 					    FuncRequest(toc_list[i].action())));
+			// separator after the menu heading
+			if (toc_list[i].depth() < depth)
+				add(MenuItem(MenuItem::Separator));
 		}
 	} else {
 		size_t pos = from;
@@ -1255,7 +1258,7 @@ void MenuDefinition::expandToc2(Toc const & toc_list,
 				++new_pos;
 
 			QString label(4 * max(0, toc_list[pos].depth() - depth), ' ');
-			label += limitStringLength(toc_list[pos].str());
+			label += limitStringLength(toc_list[pos].asString());
 			if (toc_list[pos].depth() == depth) {
 				label += '|';
 			    if (shortcut_count < 9) {
@@ -1285,12 +1288,10 @@ void MenuDefinition::expandToc(Buffer const * buf)
 	// all MenuItem constructors and to expandToc2. However, we
 	// know that all the entries in a TOC will be have status_ ==
 	// OK, so we avoid this unnecessary overhead (JMarc)
-
 	if (!buf) {
-		add(MenuItem(MenuItem::Info, qt_("<No Document Open>")));
+		add(MenuItem(MenuItem::Info, qt_("(No Document Open)")));
 		return;
 	}
-
 	// Add an entry for the master doc if this is a child doc
 	Buffer const * const master = buf->masterBuffer();
 	if (buf != master) {
@@ -1301,68 +1302,36 @@ void MenuDefinition::expandToc(Buffer const * buf)
 	}
 
 	MenuDefinition other_lists;
-
 	FloatList const & floatlist = buf->params().documentClass().floats();
 	TocList const & toc_list = buf->tocBackend().tocs();
 	TocList::const_iterator cit = toc_list.begin();
 	TocList::const_iterator end = toc_list.end();
 	for (; cit != end; ++cit) {
-		// Handle this later
-		if (cit->first == "tableofcontents")
+		// Handle table of contents later
+		if (cit->first == "tableofcontents" || cit->second->empty())
 			continue;
-
 		MenuDefinition submenu;
-		if (floatlist.typeExist(cit->first)) {
-			TocIterator ccit = cit->second->begin();
-			TocIterator eend = cit->second->end();
-			for (; ccit != eend; ++ccit) {
-				if (0 == ccit->depth()) {// omit subfloats
-					submenu.add(MenuItem(MenuItem::Command,
-										 limitStringLength(ccit->str()) + '|',
-										 FuncRequest(ccit->action())));
-				}
-			}
-
-			FuncRequest f(LFUN_DIALOG_SHOW, "toc " + cit->first);
-			submenu.add(MenuItem(MenuItem::Separator));
-			submenu.add(MenuItem(MenuItem::Command, qt_("Open Navigator..."), f));
-			MenuItem item(MenuItem::Submenu, guiName(cit->first, buf->params()));
-			// deserves to be in the main menu.
-			item.setSubmenu(submenu);
+		// "Open outliner..." entry
+		FuncRequest f(LFUN_DIALOG_SHOW, "toc " + cit->first);
+		submenu.add(MenuItem(MenuItem::Command, qt_("Open outliner..."), f));
+		submenu.add(MenuItem(MenuItem::Separator));
+		// add entries
+		submenu.expandToc2(* cit->second, 0, cit->second->size(), 0);
+		MenuItem item(MenuItem::Submenu, guiName(cit->first, buf->params()));
+		item.setSubmenu(submenu);
+		// deserves to be in the main menu?
+		if (floatlist.typeExist(cit->first) || cit->first == "child")
 			add(item);
-		} else {
-			if (cit->second->size() >= 30) {
-				// FIXME: the behaviour of the interface should not change
-				// arbitrarily. Each type should be audited to see if the list
-				// can be optimised like for floats above.
-				FuncRequest f(LFUN_DIALOG_SHOW, "toc " + cit->first);
-				submenu.add(MenuItem(MenuItem::Command, qt_("Open Navigator..."), f));
-			} else {
-				TocIterator ccit = cit->second->begin();
-				TocIterator eend = cit->second->end();
-				for (; ccit != eend; ++ccit) {
-					submenu.add(MenuItem(MenuItem::Command,
-										 limitStringLength(ccit->str()) + '|',
-										 FuncRequest(ccit->action())));
-				}
-			}
-
-			MenuItem item(MenuItem::Submenu, guiName(cit->first, buf->params()));
-			item.setSubmenu(submenu);
-			if (cit->first == "child") {
-				// deserves to be in the main menu.
-				add(item);
-			} else
-				other_lists.add(item);
-		}
+		else
+			other_lists.add(item);
 	}
 	if (!other_lists.empty()) {
 		MenuItem item(MenuItem::Submenu, qt_("Other Lists"));
 		item.setSubmenu(other_lists);
 		add(item);
 	}
-
 	// Handle normal TOC
+	add(MenuItem(MenuItem::Separator));
 	cit = toc_list.find("tableofcontents");
 	if (cit == end)
 		LYXERR(Debug::GUI, "No table of contents.");
@@ -1370,7 +1339,7 @@ void MenuDefinition::expandToc(Buffer const * buf)
 		if (!cit->second->empty())
 			expandToc2(* cit->second, 0, cit->second->size(), 0);
 		else
-			add(MenuItem(MenuItem::Info, qt_("<Empty Table of Contents>")));
+			add(MenuItem(MenuItem::Info, qt_("(Empty Table of Contents)")));
 	}
 }
 
diff --git a/src/frontends/qt4/TocModel.cpp b/src/frontends/qt4/TocModel.cpp
index 8f0e317..53475e1 100644
--- a/src/frontends/qt4/TocModel.cpp
+++ b/src/frontends/qt4/TocModel.cpp
@@ -154,7 +154,7 @@ void TocModel::updateItem(DocIterator const & dit)
 {
 	QModelIndex index = modelIndex(dit);
 	TocItem const & toc_item = tocItem(index);
-	model_->setData(index, toqstr(toc_item.str()), Qt::DisplayRole);
+	model_->setData(index, toqstr(toc_item.asString()), Qt::DisplayRole);
 	model_->setData(index, toqstr(toc_item.tooltip()), Qt::ToolTipRole);
 }
 
@@ -183,12 +183,12 @@ void TocModel::reset(shared_ptr<Toc const> toc)
 		int current_row = model_->rowCount();
 		model_->insertRows(current_row, 1);
 		QModelIndex top_level_item = model_->index(current_row, 0);
-		model_->setData(top_level_item, toqstr(item.str()), Qt::DisplayRole);
+		model_->setData(top_level_item, toqstr(item.asString()), Qt::DisplayRole);
 		model_->setData(top_level_item, index, Qt::UserRole);
 		model_->setData(top_level_item, toqstr(item.tooltip()), Qt::ToolTipRole);
 
 		LYXERR(Debug::GUI, "Toc: at depth " << item.depth()
-			<< ", added item " << item.str());
+			<< ", added item " << item.asString());
 
 		populate(index, top_level_item);
 		if (index >= end)
@@ -224,7 +224,7 @@ void TocModel::populate(unsigned int & index, QModelIndex const & parent)
 		int current_row = model_->rowCount(parent);
 		model_->insertRows(current_row, 1, parent);
 		child_item = model_->index(current_row, 0, parent);
-		model_->setData(child_item, toqstr(item.str()), Qt::DisplayRole);
+		model_->setData(child_item, toqstr(item.asString()), Qt::DisplayRole);
 		model_->setData(child_item, index, Qt::UserRole);
 		model_->setData(child_item, toqstr(item.tooltip()), Qt::ToolTipRole);
 		populate(index, child_item);
@@ -315,7 +315,7 @@ void TocModels::goTo(QString const & type, QModelIndex const & index) const
 	}
 	LASSERT(index.model() == it.value()->model(), return);
 	TocItem const item = it.value()->tocItem(index);
-	LYXERR(Debug::GUI, "TocModels::goTo " << item.str());
+	LYXERR(Debug::GUI, "TocModels::goTo " << item.asString());
 	dispatch(item.action());
 }
 
diff --git a/src/frontends/qt4/TocWidget.cpp b/src/frontends/qt4/TocWidget.cpp
index 86fa16b..bb15f31 100644
--- a/src/frontends/qt4/TocWidget.cpp
+++ b/src/frontends/qt4/TocWidget.cpp
@@ -119,16 +119,12 @@ Inset * TocWidget::itemInset() const
 
 	else if (current_type_ == "branch"
 			 || current_type_ == "index"
-			 || current_type_ == "change")
-		inset = &dit.inset();
-
-	else if (current_type_ == "table" 
+			 || current_type_ == "change"
+			 || current_type_ == "table" 
 		     || current_type_ == "listing"
-		     || current_type_ == "figure") {
-		DocIterator tmp_dit(dit);
-		tmp_dit.pop_back();
-		inset = &tmp_dit.inset();
-	}
+		     || current_type_ == "figure")
+		inset = &dit.inset();
+
 	return inset;
 }
 
@@ -157,7 +153,7 @@ bool TocWidget::getStatus(Cursor & cur, FuncRequest const & cmd,
 
 	case LFUN_LABEL_COPY_AS_REFERENCE: {
 		// For labels in math, we need to supply the label as a string
-		FuncRequest label_copy(LFUN_LABEL_COPY_AS_REFERENCE, item.asString());
+		FuncRequest label_copy(LFUN_LABEL_COPY_AS_REFERENCE, item.str());
 		if (inset)
 			return inset->getStatus(cur, label_copy, status);
 		break;
@@ -202,7 +198,7 @@ void TocWidget::doDispatch(Cursor & cur, FuncRequest const & cmd)
 
 	case LFUN_LABEL_COPY_AS_REFERENCE: {
 		// For labels in math, we need to supply the label as a string
-		FuncRequest label_copy(LFUN_LABEL_COPY_AS_REFERENCE, item.asString());
+		FuncRequest label_copy(LFUN_LABEL_COPY_AS_REFERENCE, item.str());
 		if (inset)
 			inset->dispatch(cur, label_copy);
 		break;
-- 
2.1.4

Reply via email to