Hi,

the attached patch fixes three toolbar problems:

- The toolbar name couldn't be translated in a user message
- A faulty stdtoolbars.ui resulted in a failed assertion
- "toolbar-toggle foobla" resulted in a state change for
  some dummy toolbar rather than a reasonable error message

Unfortunately, the patch looks more complicated than it actually is. The larger changes are mainly caused by code indentation.

One thing that confused me is the fact there are two toolbars ("toolbars" and "usedtoolbars") in class ToolbarBackend. This patch takes this into account by using proper method names.

Comments? OK?

Michael
Index: src/frontends/LyXView.h
===================================================================
--- src/frontends/LyXView.h	(Revision 18950)
+++ src/frontends/LyXView.h	(Arbeitskopie)
@@ -157,8 +157,8 @@
 
 	/// update the toolbar
 	void updateToolbars();
-	/// get toolbar state
-	ToolbarInfo::Flags getToolbarState(std::string const & name);
+	/// get toolbar info
+	ToolbarInfo * getToolbarInfo(std::string const & name) const;
 	/// toggle toolbar state
 	void toggleToolbarState(std::string const & name, bool allowauto);
 	/// update the menubar
Index: src/frontends/Toolbars.cpp
===================================================================
--- src/frontends/Toolbars.cpp	(Revision 18950)
+++ src/frontends/Toolbars.cpp	(Arbeitskopie)
@@ -169,58 +169,43 @@
 }
 
 
-ToolbarInfo::Flags Toolbars::getToolbarState(string const & name)
+ToolbarInfo * Toolbars::getToolbarInfo(string const & name)
 {
-	ToolbarBackend::Toolbars::const_iterator cit = toolbarbackend.begin();
-	ToolbarBackend::Toolbars::const_iterator end = toolbarbackend.end();
-
-	for (; cit != end; ++cit) {
-		if (cit->name == name)
-			return cit->flags;
-	}
-
-	LYXERR(Debug::GUI) << "Toolbar::display: no toolbar named "
-		<< name << endl;
-
-	// return dummy for msvc
-	return ToolbarInfo::OFF;
+	return toolbarbackend.getUsedToolbarInfo(name);
 }
 
 
 void Toolbars::toggleToolbarState(string const & name, bool allowauto)
 {
-	ToolbarBackend::Toolbars::iterator cit = toolbarbackend.begin();
-	ToolbarBackend::Toolbars::iterator end = toolbarbackend.end();
+	ToolbarInfo * tbi = toolbarbackend.getUsedToolbarInfo(name);
 
-	for (; cit != end; ++cit) {
-		if (cit->name == name) {
-			int flags = cit->flags;
-			// off -> on
-			if (flags & ToolbarInfo::OFF) {
-				TurnOffFlag(OFF);
-				TurnOnFlag(ON);
-			// auto -> off
-			} else if (flags & ToolbarInfo::AUTO) {
-				TurnOffFlag(AUTO);
-				TurnOnFlag(OFF);
-			} else if (allowauto 
-				   && ((flags & ToolbarInfo::MATH) 
-				       || (flags & ToolbarInfo::TABLE)
-				       || (flags & ToolbarInfo::REVIEW))) {
-				// for math etc, toggle from on -> auto
-				TurnOffFlag(ON);
-				TurnOnFlag(AUTO);
-			} else {
-				// for others, toggle from on -> off
-				TurnOffFlag(ON);
-				TurnOnFlag(OFF);
-			}
-			cit->flags = static_cast<ToolbarInfo::Flags>(flags);
-			return;
+	if (tbi != 0) {
+		int flags = tbi->flags;
+		// off -> on
+		if (flags & ToolbarInfo::OFF) {
+			TurnOffFlag(OFF);
+			TurnOnFlag(ON);
+		// auto -> off
+		} else if (flags & ToolbarInfo::AUTO) {
+			TurnOffFlag(AUTO);
+			TurnOnFlag(OFF);
+		} else if (allowauto 
+			   && ((flags & ToolbarInfo::MATH) 
+			       || (flags & ToolbarInfo::TABLE)
+			       || (flags & ToolbarInfo::REVIEW))) {
+			// for math etc, toggle from on -> auto
+			TurnOffFlag(ON);
+			TurnOnFlag(AUTO);
+		} else {
+			// for others, toggle from on -> off
+			TurnOffFlag(ON);
+			TurnOnFlag(OFF);
 		}
+		tbi->flags = static_cast<ToolbarInfo::Flags>(flags);
+	} else {
+		LYXERR(Debug::GUI) << "Toolbar::display: no toolbar named "
+			<< name << endl;
 	}
-	LYXERR(Debug::GUI) << "Toolbar::display: no toolbar named "
-		<< name << endl;
 }
 #undef TurnOnFlag
 #undef TurnOffFlag
Index: src/frontends/qt4/QLToolbar.cpp
===================================================================
--- src/frontends/qt4/QLToolbar.cpp	(Revision 18950)
+++ src/frontends/qt4/QLToolbar.cpp	(Arbeitskopie)
@@ -214,26 +214,30 @@
 		IconPalette * panel = new IconPalette(tb);
 		panel->setWindowTitle(qt_(to_ascii(item.label_)));
 		connect(this, SIGNAL(updated()), panel, SLOT(updateParent()));
-		ToolbarInfo const & tbinfo = toolbarbackend.getToolbar(item.name_);
-		ToolbarInfo::item_iterator it = tbinfo.items.begin();
-		ToolbarInfo::item_iterator const end = tbinfo.items.end();
-		for (; it != end; ++it)
-			if (!getStatus(it->func_).unknown()) {
-				Action * action = new Action(owner_,
-					getIcon(it->func_),
-					it->label_,
-					it->func_,
-					it->label_);
-				panel->addButton(action);
-				ActionVector.push_back(action);
-				// use the icon of first action for the toolbar button
-				if (it == tbinfo.items.begin())
-					tb->setIcon(QPixmap(getIcon(it->func_).c_str()));
-			}
-		tb->setCheckable(true);
-		connect(tb, SIGNAL(clicked(bool)), panel, SLOT(setVisible(bool)));
-		connect(panel, SIGNAL(visible(bool)), tb, SLOT(setChecked(bool)));
-		addWidget(tb);
+		ToolbarInfo * tbinfo = toolbarbackend.getAllToolbarInfo(item.name_);
+		if (tbinfo != 0) {
+			ToolbarInfo::item_iterator it = tbinfo->items.begin();
+			ToolbarInfo::item_iterator const end = tbinfo->items.end();
+			for (; it != end; ++it)
+				if (!getStatus(it->func_).unknown()) {
+					Action * action = new Action(owner_,
+						getIcon(it->func_),
+						it->label_,
+						it->func_,
+						it->label_);
+					panel->addButton(action);
+					ActionVector.push_back(action);
+					// use the icon of first action for the toolbar button
+					if (it == tbinfo->items.begin())
+						tb->setIcon(QPixmap(getIcon(it->func_).c_str()));
+				}
+			tb->setCheckable(true);
+			connect(tb, SIGNAL(clicked(bool)), panel, SLOT(setVisible(bool)));
+			connect(panel, SIGNAL(visible(bool)), tb, SLOT(setChecked(bool)));
+			addWidget(tb);
+		} else {
+			lyxerr << "Unknown toolbar " << item.name_ << endl;
+		}
 		break;
 		}
 	case ToolbarItem::POPUPMENU: {
@@ -251,21 +255,25 @@
 		m->setWindowTitle(qt_(to_ascii(item.label_)));
 		m->setTearOffEnabled(true);
 		connect(this, SIGNAL(updated()), m, SLOT(updateParent()));
-		ToolbarInfo const & tbinfo = toolbarbackend.getToolbar(item.name_);
-		ToolbarInfo::item_iterator it = tbinfo.items.begin();
-		ToolbarInfo::item_iterator const end = tbinfo.items.end();
-		for (; it != end; ++it)
-			if (!getStatus(it->func_).unknown()) {
-				Action * action = new Action(owner_,
-					getIcon(it->func_, false),
-					it->label_,
-					it->func_,
-					it->label_);
-				m->add(action);
-				ActionVector.push_back(action);
-			}
-		tb->setMenu(m);
-		addWidget(tb);
+		ToolbarInfo * tbinfo = toolbarbackend.getAllToolbarInfo(item.name_);
+		if (tbinfo != 0) {
+			ToolbarInfo::item_iterator it = tbinfo->items.begin();
+			ToolbarInfo::item_iterator const end = tbinfo->items.end();
+			for (; it != end; ++it)
+				if (!getStatus(it->func_).unknown()) {
+					Action * action = new Action(owner_,
+						getIcon(it->func_, false),
+						it->label_,
+						it->func_,
+						it->label_);
+					m->add(action);
+					ActionVector.push_back(action);
+				}
+			tb->setMenu(m);
+			addWidget(tb);
+		} else {
+			lyxerr << "Unknown toolbar " << item.name_ << endl;
+		}
 		break;
 		}
 	case ToolbarItem::COMMAND: {
Index: src/frontends/LyXView.cpp
===================================================================
--- src/frontends/LyXView.cpp	(Revision 18950)
+++ src/frontends/LyXView.cpp	(Arbeitskopie)
@@ -406,9 +406,9 @@
 }
 
 
-ToolbarInfo::Flags LyXView::getToolbarState(string const & name)
+ToolbarInfo * LyXView::getToolbarInfo(string const & name) const
 {
-	return toolbars_->getToolbarState(name);
+	return toolbars_->getToolbarInfo(name);
 }
 
 
Index: src/frontends/Toolbars.h
===================================================================
--- src/frontends/Toolbars.h	(Revision 18950)
+++ src/frontends/Toolbars.h	(Arbeitskopie)
@@ -90,8 +90,8 @@
 	/// Show/hide the named toolbar.
 	void display(std::string const & name, bool show);
 
-	/// get toolbar state (on/off/auto)
-	ToolbarInfo::Flags getToolbarState(std::string const & name);
+	/// get toolbar info
+	ToolbarInfo * getToolbarInfo(std::string const & name);
 
 	/** toggle the state of toolbars (on/off/auto). Skip "auto"
 	 * when allowauto is false.
Index: src/LyXFunc.cpp
===================================================================
--- src/LyXFunc.cpp	(Revision 18950)
+++ src/LyXFunc.cpp	(Arbeitskopie)
@@ -1774,18 +1774,22 @@
 			string const name = cmd.getArg(0);
 			bool const allowauto = cmd.getArg(1) == "allowauto";
 			lyx_view_->toggleToolbarState(name, allowauto);
-			ToolbarInfo::Flags const flags = 
-				lyx_view_->getToolbarState(name);
-			docstring state;
-			if (flags & ToolbarInfo::ON)
-				state = _("on");
-			else if (flags & ToolbarInfo::OFF)
-				state = _("off");
-			else if (flags & ToolbarInfo::AUTO)
-				state = _("auto");
+			ToolbarInfo * tbi = lyx_view_->getToolbarInfo(name);
+			if (tbi != 0) {
+				docstring state;
+				if (tbi->flags & ToolbarInfo::ON)
+					state = _("on");
+				else if (tbi->flags & ToolbarInfo::OFF)
+					state = _("off");
+				else if (tbi->flags & ToolbarInfo::AUTO)
+					state = _("auto");
 
-			setMessage(bformat(_("Toolbar \"%1$s\" state set to %2$s"), 
-						   from_ascii(name), state));
+				setMessage(bformat(_("Toolbar \"%1$s\" state set to %2$s"), 
+				                   _(tbi->gui_name), state));
+			} else {
+				setMessage(bformat(_("Unknown toolbar \"%1$s\""),
+				                   from_utf8(name)));
+			}
 			break;
 		}
 
Index: src/ToolbarBackend.cpp
===================================================================
--- src/ToolbarBackend.cpp	(Revision 18950)
+++ src/ToolbarBackend.cpp	(Arbeitskopie)
@@ -328,24 +328,23 @@
 }
 
 
-ToolbarInfo const & ToolbarBackend::getToolbar(string const & name) const
+ToolbarInfo * ToolbarBackend::getAllToolbarInfo(string const & name)
 {
-	Toolbars::const_iterator cit = find_if(toolbars.begin(), toolbars.end(), ToolbarNamesEqual(name));
-	if (cit == toolbars.end())
-		lyxerr << "No toolbar named " << name << endl;
-	BOOST_ASSERT(cit != toolbars.end());
-	return (*cit);
+	Toolbars::iterator it = find_if(toolbars.begin(), toolbars.end(), ToolbarNamesEqual(name));
+	if (it == toolbars.end()) {
+		return 0;
+	}
+	return &(*it);
 }
 
 
-ToolbarInfo & ToolbarBackend::getToolbar(string const & name)
+ToolbarInfo * ToolbarBackend::getUsedToolbarInfo(string const &name)
 {
-	Toolbars::iterator it = find_if(toolbars.begin(), toolbars.end(), ToolbarNamesEqual(name));
-	if (it == toolbars.end())
-		lyxerr << "No toolbar named " << name << endl;
-	BOOST_ASSERT(it != toolbars.end());
-	return (*it);
+	Toolbars::iterator it = find_if(usedtoolbars.begin(), usedtoolbars.end(), ToolbarNamesEqual(name));
+	if (it == usedtoolbars.end()) {
+		return 0;
+	}
+	return &(*it);
 }
 
-
 } // namespace lyx
Index: src/ToolbarBackend.h
===================================================================
--- src/ToolbarBackend.h	(Revision 18950)
+++ src/ToolbarBackend.h	(Arbeitskopie)
@@ -126,10 +126,11 @@
 
 	/// read ui toolbar settings
 	void readToolbarSettings(Lexer &);
+
 	///
-	ToolbarInfo const & getToolbar(std::string const & name) const;
+	ToolbarInfo * getAllToolbarInfo(std::string const & name);
 	///
-	ToolbarInfo & getToolbar(std::string const & name);
+	ToolbarInfo * getUsedToolbarInfo(std::string const & name);
 
 private:
 	/// all the toolbars

Reply via email to