Michael Gerz wrote:
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.
ok by me. i don't really like all these checks: if (tbi != 0), but don't
know what a nice solution would be. instead of indenting i would always
try to return or break early though (see below)
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.
maybe you can rename toolbars to toolbardefinitions, and getToolbar() to
getToolbarDefinition() or something like that?
Comments? OK?
some comments below, but then ok afaiac
> 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);
> [snip]
> + if (tbi != 0) {
i would prefer to return early
> Index: src/frontends/qt4/QLToolbar.cpp
> ===================================================================
> --- src/frontends/qt4/QLToolbar.cpp (Revision 18950)
> +++ src/frontends/qt4/QLToolbar.cpp (Arbeitskopie)
> [snip]
> + if (tbinfo != 0) {
same here, but to we really need these?
> [snip]
> case ToolbarItem::POPUPMENU: {
> [snip]
> + if (tbinfo != 0) {
ditto
> Index: src/LyXFunc.cpp
> ===================================================================
> --- src/LyXFunc.cpp (Revision 18950)
> +++ src/LyXFunc.cpp (Arbeitskopie)
> @@ -1774,18 +1774,22 @@
> [snip]
> + if (tbi != 0) {
ditto
> 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);
> }
+ if (it == toolbars.end())
+ return 0;
> -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);
> }
+ if (it == toolbars.end())
+ return 0;
> 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
why no longer const?
