Workbench drop down list is long, too long.

[wmayer]

• Please add a license notice to QListWidgetWithDragging.h

• Code: Select all

  enum Type { Toolbar, Toolboxbar };

  This can be removed from DlgWorkbenches as it is of no use there.
• You don't need the extra class DlgWorkbenches. Just move the relevant stuff to DlgWorkbenchesImp
• Build failure in DlgWorkbenchesImp::renameCustomToolbar because of the missing curly brace

[PrzemoF]

Thanks, I'll add the licence. There a a lot of empty/spare definitions, but I'm not sure what can be safely removed.

[wmayer]

Found a bug: From the list of enabled workbenches you can drag an item to another instance of FreeCAD. This way it's possible to have a duplicate workbench.

[wmayer]

Here is a list of what can be removed:

• addCustomToolbar
• removeCustomToolbar (2x)
• addCustomCommand
• removeCustomCommand
• moveUpCustomCommand
• moveDownCustomCommand
• enum Type

The changeEvent() should be implemented:

Code: Select all

void DlgWorkbenchesImp::changeEvent(QEvent *e)
{
    if (e->type() == QEvent::LanguageChange) {
        retranslateUi(this);
    }
    else {
        QWidget::changeEvent(e);
    }
}

[PrzemoF]

Thanks for the review and the fixes! I got it all problems solved, but I'm not happy with the quality of the code - there are some repetitive code sections that I'd like to remove, but I'm struggling with C++ classes. Can you take a look at the comments that I left in-line and let me know if what I want to do makes sense?
A new version: https://github.com/PrzemoF/FreeCAD_sf_m ... benches_v4

[PrzemoF]

A fully functional version: https://github.com/PrzemoF/FreeCAD_sf_m ... benches_v5
Demo: http://firszt.eu/FreeCAD/FreeCAD_workbenches_v5.webm

Supported:
- adding, removing or changing order with buttons
- adding, removing or changing order by dragging, but only within one FreeCAD instance
- adding all workbenches to "enabled" group with one button click
- sorting "enabled" group with one button click
- saving/restoring workbenches to Preferences
-"disabled" group is always sorted and user cannot change the order

Do we need a button to "remove all from enabled"? It would be trivial change, but I don't want to overload the Gui with useless features.

Potential pitfalls:
- Leaving None as only active workbench or removing all workbenches as it sets None wb as the only active workbench.Activating None wb removes Tools menu, so there is no easy way back. There are 2 ways to recover: setting start workbench in Preferences to something else not "None" - this restores Tools menu. Another way is to remove "Workbenches" group from Preferences file (user.cfg), so all workbenches are added to "enabled" group.

[wmayer]

make function set_list_properites(QListWidgetCustom)?

You can also add a method to QListWidgetCustom.

"ALL" should be a "magic" string defined in class, but I don't know yet how to do it

You can make this a member of DlgWorkbenchesImp which you then set in the constructor to "ALL".

[PrzemoF]

Thanks! So I have to extend QListWidgetCustom into a proper class with constructor, destructor in separate .cpp file and so on?
I'll add "ALL" string as suggested - it's used in a few places, so it's better to have one definition to avoid future problems.

I'll have to rethink how to use load_enabled_workbenches in src/Gui/Actions.cpp - I was getting some missing object error. Currently there are identical code in 2 places to get the workbenches from preferences. I src/Gui/Action.cpp

Code: Select all

   
QStringList items = Application::Instance->workbenches();
[..]
hGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Workbenches");
enabled_wbs = QString::fromStdString(hGrp->GetASCII("Enabled", "ALL").c_str());
enabled_wbs_list = enabled_wbs.split(QLatin1String(","), QString::SkipEmptyParts);
[..]
const QString all_workbenches = QString::fromAscii("ALL");
if (enabled_wbs_list.at(0) == all_workbenches) {
    items.removeFirst();
    for (QStringList::Iterator it = items.begin(); it != items.end(); ++it) {
        enabled_wbs_list.append(*it);
    }
    enabled_wbs_list.sort();
}

and in src/Gui/DlgWorkbenchesIp.cpp:

Code: Select all

QStringList DlgWorkbenchesImp::load_enabled_workbenches()
{
    QString enabled_wbs;
    QStringList enabled_wbs_list;
    ParameterGrp::handle hGrp;

    hGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Workbenches");
    enabled_wbs = QString::fromStdString(hGrp->GetASCII("Enabled", "ALL").c_str());
    enabled_wbs_list = enabled_wbs.split(QLatin1String(","), QString::SkipEmptyParts);
    return enabled_wbs_list;
}

with the call:

Code: Select all

QStringList enabled_wbs_list = load_enabled_workbenches();
QStringList workbenches = Application::Instance->workbenches();

int i = 0;
QPixmap px;
QString mt;
if (enabled_wbs_list.at(0) == QString::fromAscii("ALL")) {
    enabled_wbs_list.removeFirst();
    for (QStringList::Iterator it = workbenches.begin(); it != workbenches.end(); ++it) {
        enabled_wbs_list.append(*it);
    }
    enabled_wbs_list.sort();
}

[wmayer]

So I have to extend QListWidgetCustom into a proper class with constructor, destructor in separate .cpp file and so on?

Yes.

[PrzemoF]

@wmayer: thanks for being so patient with my C++ and for all the tests, coments and hints!
The final version is here: https://github.com/PrzemoF/FreeCAD_sf_m ... benches_v6
I'm not going to make any changes unless there is something wrong, broken or missing.
I'll send a pull request after a few days to give some time for tests.

@all, thanks again for your input!