Abdelrazak Younes wrote:
Jean-Marc Lasgouttes wrote:
"Abdelrazak" == Abdelrazak Younes <[EMAIL PROTECTED]> writes:

Abdelrazak> Dear Jean-Marc, dear all, This change seems natural to me
Abdelrazak> as none of the LyXView features are used.

The expandFoo changes are correct, but I tend to think that the
following is not the best:
-    Menu & add(MenuItem const &, LyXView const * view = 0);
+    Menu & add(MenuItem const &, Buffer const * buf = 0);

What about removing the second parameter altogether? What are the
cases when add() can be called with view == 0?


I do not think the buffer parameter has any use here.

The only use is there:

Menu & Menu::add(MenuItem const & i, Buffer const * buf)
{
    if (!buf) {
        items_.push_back(i);
        return *this;
    }


You can see here that the passed Buffer (or LyXView in the old code) is just there to differentiate between two methods. Splitting the code is maybe cleaner. I'll send another patch in a minute.

What do you think of the attached. I even simplified some expand functions...

Abdel.


Index: frontends/gtk/GMenubar.C
===================================================================
--- frontends/gtk/GMenubar.C    (revision 15153)
+++ frontends/gtk/GMenubar.C    (working copy)
@@ -159,7 +159,7 @@
        Menu::const_iterator end;
        if(!item->submenuname().empty()) {
                fmenu = &menubackend.getMenu(item->submenuname());
-               menubackend.expand(*fmenu, lyxmenu->getBackMenu(), view_);
+               menubackend.expand(*fmenu, lyxmenu->getBackMenu(), 
view_->buffer());
                i = lyxmenu->getBackMenu().begin();
                end = lyxmenu->getBackMenu().end();
        } else {
Index: frontends/qt3/QLPopupMenu.C
===================================================================
--- frontends/qt3/QLPopupMenu.C (revision 15153)
+++ frontends/qt3/QLPopupMenu.C (working copy)
@@ -163,7 +163,7 @@
        clear();
        Menu tomenu;
        Menu const frommenu = owner_->backend().getMenu(name_);
-       owner_->backend().expand(frommenu, tomenu, owner_->view());
+       owner_->backend().expand(frommenu, tomenu, owner_->view()->buffer());
        populate(&tomenu);
 #ifdef Q_WS_MACX
        /* The qt/mac menu code has a very silly hack that
Index: frontends/qt4/QLMenubar.C
===================================================================
--- frontends/qt4/QLMenubar.C   (revision 15153)
+++ frontends/qt4/QLMenubar.C   (working copy)
@@ -60,7 +60,7 @@
        //      for (; m != end; ++m) {
 
        Menu menu;
-       menubackend_.expand(menubackend_.getMenubar(), menu, owner_);
+       menubackend_.expand(menubackend_.getMenubar(), menu, owner_->buffer());
 
        Menu::const_iterator m = menu.begin();
        Menu::const_iterator end = menu.end();
@@ -81,7 +81,7 @@
                }
 
                Menu menu;
-               menubackend_.expand(menubackend_.getMenubar(), menu, owner_);
+               menubackend_.expand(menubackend_.getMenubar(), menu, 
owner_->buffer());
 
                QLPopupMenu * qMenu = new QLPopupMenu(this, *m, true);
                owner_->menuBar()->addMenu(qMenu);
Index: frontends/qt4/QLPopupMenu.C
===================================================================
--- frontends/qt4/QLPopupMenu.C (revision 15153)
+++ frontends/qt4/QLPopupMenu.C (working copy)
@@ -75,7 +75,7 @@
                return;
 
        Menu const & fromLyxMenu = owner_->backend().getMenu(name_);
-       owner_->backend().expand(fromLyxMenu, topLevelMenu_, owner_->view());
+       owner_->backend().expand(fromLyxMenu, topLevelMenu_, 
owner_->view()->buffer());
 
        if (!owner_->backend().hasMenu(topLevelMenu_.name())) {
                lyxerr[Debug::GUI] << "\tWARNING: menu seems empty" << 
lyx::to_utf8(topLevelMenu_.name()) << endl;
Index: MenuBackend.C
===================================================================
--- MenuBackend.C       (revision 15153)
+++ MenuBackend.C       (working copy)
@@ -37,7 +37,7 @@
 #include "lyxlex.h"
 #include "toc.h"
 
-#include "frontends/LyXView.h"
+#include "frontends/Application.h"
 
 #include "support/filetools.h"
 #include "support/lstrings.h"
@@ -157,18 +157,20 @@
 }
 
 
-Menu & Menu::add(MenuItem const & i, LyXView const * view)
+Menu & Menu::add(MenuItem const & i)
 {
-       if (!view) {
-               items_.push_back(i);
-               return *this;
-       }
+       items_.push_back(i);
+       return *this;
+}
 
+
+Menu & Menu::addWithCheck(MenuItem const & i)
+{
        switch (i.kind()) {
 
        case MenuItem::Command: {
                FuncStatus status =
-                       view->getLyXFunc().getStatus(i.func());
+                       theApp->lyxFunc().getStatus(i.func());
                if (status.unknown()
                    || (!status.enabled() && i.optional()))
                        break;
@@ -429,7 +431,7 @@
 }
 
 
-void expandLastfiles(Menu & tomenu, LyXView const * view)
+void expandLastfiles(Menu & tomenu)
 {
        lyx::Session::LastFiles const & lf = LyX::cref().session().lastFiles();
        lyx::Session::LastFiles::const_iterator lfit = lf.begin();
@@ -440,19 +442,19 @@
                docstring const label = convert<docstring>(ii) + 
lyx::from_ascii(". ")
                        + makeDisplayPath((*lfit), 30)
                        + char_type('|') + convert<docstring>(ii);
-               tomenu.add(MenuItem(MenuItem::Command, label, 
FuncRequest(LFUN_FILE_OPEN, (*lfit))), view);
+               tomenu.addWithCheck(MenuItem(MenuItem::Command, label, 
FuncRequest(LFUN_FILE_OPEN, (*lfit))));
        }
 }
 
 
-void expandDocuments(Menu & tomenu, LyXView const * view)
+void expandDocuments(Menu & tomenu)
 {
        typedef vector<string> Strings;
        Strings const names = bufferlist.getFileNames();
 
        if (names.empty()) {
-               tomenu.add(MenuItem(MenuItem::Command, _("No Documents Open!"),
-                                   FuncRequest(LFUN_NOACTION)), view);
+               tomenu.addWithCheck(MenuItem(MenuItem::Command, _("No Documents 
Open!"),
+                                   FuncRequest(LFUN_NOACTION)));
                return;
        }
 
@@ -463,18 +465,17 @@
                docstring label = makeDisplayPath(*docit, 20);
                if (ii < 10)
                        label = convert<docstring>(ii) + lyx::from_ascii(". ") 
+ label + char_type('|') + convert<docstring>(ii);
-               tomenu.add(MenuItem(MenuItem::Command, label, 
FuncRequest(LFUN_BUFFER_SWITCH, *docit)), view);
+               tomenu.addWithCheck(MenuItem(MenuItem::Command, label, 
FuncRequest(LFUN_BUFFER_SWITCH, *docit)));
        }
 }
 
 
-void expandFormats(MenuItem::Kind kind, Menu & tomenu, LyXView const * view)
+void expandFormats(MenuItem::Kind kind, Menu & tomenu, Buffer const * buf)
 {
-       if (!view->buffer() && kind != MenuItem::ImportFormats) {
-               tomenu.add(MenuItem(MenuItem::Command,
+       if (!buf && kind != MenuItem::ImportFormats) {
+               tomenu.addWithCheck(MenuItem(MenuItem::Command,
                                    _("No Documents Open!"),
-                                   FuncRequest(LFUN_NOACTION)),
-                                   view);
+                                   FuncRequest(LFUN_NOACTION)));
                return;
        }
 
@@ -488,15 +489,15 @@
                action = LFUN_BUFFER_IMPORT;
                break;
        case MenuItem::ViewFormats:
-               formats = Exporter::getExportableFormats(*view->buffer(), true);
+               formats = Exporter::getExportableFormats(*buf, true);
                action = LFUN_BUFFER_VIEW;
                break;
        case MenuItem::UpdateFormats:
-               formats = Exporter::getExportableFormats(*view->buffer(), true);
+               formats = Exporter::getExportableFormats(*buf, true);
                action = LFUN_BUFFER_UPDATE;
                break;
        default:
-               formats = Exporter::getExportableFormats(*view->buffer(), 
false);
+               formats = Exporter::getExportableFormats(*buf, false);
                action = LFUN_BUFFER_EXPORT;
        }
        sort(formats.begin(), formats.end(), compare_format());
@@ -529,80 +530,74 @@
                if (!(*fit)->shortcut().empty())
                        label += char_type('|') + 
lyx::from_utf8((*fit)->shortcut());
 
-               tomenu.add(MenuItem(MenuItem::Command, label,
-                                   FuncRequest(action, (*fit)->name())),
-                          view);
+               tomenu.addWithCheck(MenuItem(MenuItem::Command, label,
+                                   FuncRequest(action, (*fit)->name())));
        }
 }
 
 
-void expandFloatListInsert(Menu & tomenu, LyXView const * view)
+void expandFloatListInsert(Menu & tomenu, Buffer const * buf)
 {
-       if (!view->buffer()) {
-               tomenu.add(MenuItem(MenuItem::Command,
+       if (!buf) {
+               tomenu.addWithCheck(MenuItem(MenuItem::Command,
                                    _("No Documents Open!"),
-                                   FuncRequest(LFUN_NOACTION)),
-                          view);
+                                   FuncRequest(LFUN_NOACTION)));
                return;
        }
 
        FloatList const & floats =
-               view->buffer()->params().getLyXTextClass().floats();
+               buf->params().getLyXTextClass().floats();
        FloatList::const_iterator cit = floats.begin();
        FloatList::const_iterator end = floats.end();
        for (; cit != end; ++cit) {
-               tomenu.add(MenuItem(MenuItem::Command,
+               tomenu.addWithCheck(MenuItem(MenuItem::Command,
                                    _(cit->second.listName()),
                                    FuncRequest(LFUN_FLOAT_LIST,
-                                               cit->second.type())),
-                          view);
+                                               cit->second.type())));
        }
 }
 
 
-void expandFloatInsert(Menu & tomenu, LyXView const * view)
+void expandFloatInsert(Menu & tomenu, Buffer const * buf)
 {
-       if (!view->buffer()) {
-               tomenu.add(MenuItem(MenuItem::Command,
+       if (!buf) {
+               tomenu.addWithCheck(MenuItem(MenuItem::Command,
                                    _("No Documents Open!"),
-                                   FuncRequest(LFUN_NOACTION)),
-                          view);
+                                   FuncRequest(LFUN_NOACTION)));
                return;
        }
 
        FloatList const & floats =
-               view->buffer()->params().getLyXTextClass().floats();
+               buf->params().getLyXTextClass().floats();
        FloatList::const_iterator cit = floats.begin();
        FloatList::const_iterator end = floats.end();
        for (; cit != end; ++cit) {
                // normal float
                docstring const label = _(cit->second.name());
-               tomenu.add(MenuItem(MenuItem::Command, label,
+               tomenu.addWithCheck(MenuItem(MenuItem::Command, label,
                                    FuncRequest(LFUN_FLOAT_INSERT,
-                                               cit->second.type())),
-                          view);
+                                               cit->second.type())));
        }
 }
 
 
-void expandCharStyleInsert(Menu & tomenu, LyXView const * view)
+void expandCharStyleInsert(Menu & tomenu, Buffer const * buf)
 {
-       if (!view->buffer()) {
-               tomenu.add(MenuItem(MenuItem::Command,
+       if (!buf) {
+               tomenu.addWithCheck(MenuItem(MenuItem::Command,
                                    _("No Documents Open!"),
-                                   FuncRequest(LFUN_NOACTION)),
-                          view);
+                                   FuncRequest(LFUN_NOACTION)));
                return;
        }
        CharStyles & charstyles =
-               view->buffer()->params().getLyXTextClass().charstyles();
+               buf->params().getLyXTextClass().charstyles();
        CharStyles::iterator cit = charstyles.begin();
        CharStyles::iterator end = charstyles.end();
        for (; cit != end; ++cit) {
                docstring const label = lyx::from_utf8(cit->name);
-               tomenu.add(MenuItem(MenuItem::Command, label,
+               tomenu.addWithCheck(MenuItem(MenuItem::Command, label,
                                    FuncRequest(LFUN_CHARSTYLE_INSERT,
-                                               cit->name)), view);
+                                               cit->name)));
        }
 }
 
@@ -667,20 +662,17 @@
 }
 
 
-void expandToc(Menu & tomenu, LyXView const * view)
+void expandToc(Menu & tomenu, Buffer const * buf)
 {
-       // To make things very cleanly, we would have to pass view to
+       // To make things very cleanly, we would have to pass buf to
        // 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)
 
-
-       Buffer const * buf = view->buffer();
        if (!buf) {
-               tomenu.add(MenuItem(MenuItem::Command,
+               tomenu.addWithCheck(MenuItem(MenuItem::Command,
                                    _("No Documents Open!"),
-                                   FuncRequest(LFUN_NOACTION)),
-                          view);
+                                   FuncRequest(LFUN_NOACTION)));
                return;
        }
 
@@ -712,23 +704,22 @@
        // Handle normal TOC
        cit = toc_list.find("TOC");
        if (cit == end) {
-               tomenu.add(MenuItem(MenuItem::Command,
+               tomenu.addWithCheck(MenuItem(MenuItem::Command,
                                    _("No Table of contents"),
-                                   FuncRequest()),
-                          view);
+                                   FuncRequest()));
        } else {
                expandToc2(tomenu, cit->second, 0, cit->second.size(), 0);
        }
 }
 
 
-void expandPasteRecent(Menu & tomenu, LyXView const * view)
+void expandPasteRecent(Menu & tomenu, Buffer const * buf)
 {
-       if (!view || !view->buffer())
+       if (!buf)
                return;
 
        vector<string> const sel =
-               lyx::cap::availableSelections(*view->buffer());
+               lyx::cap::availableSelections(*buf);
 
        vector<string>::const_iterator cit = sel.begin();
        vector<string>::const_iterator end = sel.end();
@@ -740,12 +731,12 @@
 }
 
 
-void expandBranches(Menu & tomenu, LyXView const * view)
+void expandBranches(Menu & tomenu, Buffer const * buf)
 {
-       if (!view || !view->buffer())
+       if (!buf)
                return;
 
-       BufferParams const & params = 
view->buffer()->getMasterBuffer()->params();
+       BufferParams const & params = buf->getMasterBuffer()->params();
 
        BranchList::const_iterator cit = params.branchlist().begin();
        BranchList::const_iterator end = params.branchlist().end();
@@ -754,9 +745,9 @@
                docstring label = lyx::from_utf8(cit->getBranch());
                if (ii < 10)
                        label = convert<docstring>(ii) + lyx::from_ascii(". ") 
+ label + char_type('|') + convert<docstring>(ii);
-               tomenu.add(MenuItem(MenuItem::Command, label,
+               tomenu.addWithCheck(MenuItem(MenuItem::Command, label,
                                    FuncRequest(LFUN_BRANCH_INSERT,
-                                               cit->getBranch())), view);
+                                               cit->getBranch())));
        }
 }
 
@@ -765,7 +756,7 @@
 
 
 void MenuBackend::expand(Menu const & frommenu, Menu & tomenu,
-                        LyXView const * view) const
+                        Buffer const * buf) const
 {
        if (!tomenu.empty())
                tomenu.clear();
@@ -774,61 +765,61 @@
             cit != frommenu.end() ; ++cit) {
                switch (cit->kind()) {
                case MenuItem::Lastfiles:
-                       expandLastfiles(tomenu, view);
+                       expandLastfiles(tomenu);
                        break;
 
                case MenuItem::Documents:
-                       expandDocuments(tomenu, view);
+                       expandDocuments(tomenu);
                        break;
 
                case MenuItem::ImportFormats:
                case MenuItem::ViewFormats:
                case MenuItem::UpdateFormats:
                case MenuItem::ExportFormats:
-                       expandFormats(cit->kind(), tomenu, view);
+                       expandFormats(cit->kind(), tomenu, buf);
                        break;
 
                case MenuItem::CharStyles:
-                       expandCharStyleInsert(tomenu, view);
+                       expandCharStyleInsert(tomenu, buf);
                        break;
 
                case MenuItem::FloatListInsert:
-                       expandFloatListInsert(tomenu, view);
+                       expandFloatListInsert(tomenu, buf);
                        break;
 
                case MenuItem::FloatInsert:
-                       expandFloatInsert(tomenu, view);
+                       expandFloatInsert(tomenu, buf);
                        break;
 
                case MenuItem::PasteRecent:
-                       expandPasteRecent(tomenu, view);
+                       expandPasteRecent(tomenu, buf);
                        break;
 
                case MenuItem::Branches:
-                       expandBranches(tomenu, view);
+                       expandBranches(tomenu, buf);
                        break;
 
                case MenuItem::Toc:
-                       expandToc(tomenu, view);
+                       expandToc(tomenu, buf);
                        break;
 
                case MenuItem::Submenu: {
                        MenuItem item(*cit);
                        item.submenu(new Menu(cit->submenuname()));
                        expand(getMenu(cit->submenuname()),
-                              *item.submenu(), view);
-                       tomenu.add(item, view);
+                              *item.submenu(), buf);
+                       tomenu.addWithCheck(item);
                }
                break;
 
                case MenuItem::Separator:
-                       tomenu.add(*cit, view);
+                       tomenu.addWithCheck(*cit);
                        break;
 
                case MenuItem::Command:
                        if (!specialmenu_
                            || !specialmenu_->hasFunc(cit->func()))
-                               tomenu.add(*cit, view);
+                               tomenu.addWithCheck(*cit);
                }
        }
 
Index: MenuBackend.h
===================================================================
--- MenuBackend.h       (revision 15153)
+++ MenuBackend.h       (working copy)
@@ -21,7 +21,7 @@
 #include <vector>
 
 class LyXLex;
-class LyXView;
+class Buffer;
 class Menu;
 
 ///
@@ -145,9 +145,11 @@
        ///
        explicit Menu(lyx::docstring const & name = lyx::docstring())
                : name_(name) {}
+       /// Add the menu item unconditionally
+       Menu & add(MenuItem const &);
+       /// Operates some verification and/or before adding the menu item.
+       Menu & addWithCheck(MenuItem const &);
        ///
-       Menu & add(MenuItem const &, LyXView const * view = 0);
-       ///
        Menu & read(LyXLex &);
        ///
        lyx::docstring const & name() const { return name_; }
@@ -218,7 +220,7 @@
            ViewFormats, ExportFormats, UpdateFormats, Branches
        */
        void expand(Menu const & frommenu, Menu & tomenu,
-                   LyXView const *) const;
+                   Buffer const *) const;
        ///
        const_iterator begin() const {
                return menulist_.begin();

Reply via email to