[digikam] [Bug 377197] New: Customize grouping behaviour

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

            Bug ID: 377197
           Summary: Customize grouping behaviour
           Product: digikam
           Version: 5.5.0
          Platform: Other
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: NOR
         Component: AlbumsView-Group
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ---

Created attachment 104362
  --> https://bugs.kde.org/attachment.cgi?id=104362&action=edit
Patch to customize grouping behaviour

This extends the changes made in https://bugs.kde.org/show_bug.cgi?id=372027
As requested in the user mailing list, neither the old behaviour (disregard
grouping) nor the behaviour introduces in the issue linked above (always
operate on all group items) is ideal for all circumstances/users.

This patch makes it customizable for operations where it makes sense/is
possible. The behaviour is specifiable in the configuration in the
"Miscellaneous" part. However this does not need to be specified manually by
the user at all. By default these are set to "Ask", meaning a dialogue will pop
up when an operation is run. There one can choose whether the operation should
happen on all items in the group or not and a checkbox to remember the choice
is provided.

Also I found some dead/duplicate code related to icon and table view, which I
centralized. In the end the whole patch got quite big. I can push it to a
branch with the refactoring and grouping related code separated if you want.

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

Maik Qualmann <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #1 from Maik Qualmann <[hidden email]> ---
Simon,

I think we do not need that much. A simple "use first image of grouped or use
all images" for all actions should be enough. Let's see what Gilles says when
he has time. I have also found some errors on the right mouse button context
menu:

- Assign color label to a group, the dialog asks for each image
- Assign or remove tags no longer works
- Assign rating no longer works
- Assign flags no longer works

Maik

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]
            Summary|Customize grouping          |Customize grouping
                   |behaviour                   |behaviour [patch]

--- Comment #2 from [hidden email] ---
Simon,

Curiously, i never seen this entry until Maik comment it.

So, i will take a look when time permit, i hope while this week end...

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.

[Simon Frei]

Yes, context menu is obviously broken. I tested all kinds of things but
for unknown reasons never once did a right click. This code is also
heavily duplicated between table and icon view, I am thus refactoring
while fixing.

On 08/03/17 20:21, Maik Qualmann wrote:

> Simon,
>
> I think we do not need that much. A simple "use first image of grouped or use
> all images" for all actions should be enough. Let's see what Gilles says when
> he has time. I have also found some errors on the right mouse button context
> menu:
>
> - Assign color label to a group, the dialog asks for each image
> - Assign or remove tags no longer works
> - Assign rating no longer works
> - Assign flags no longer works
>
> Maik
>

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #3 from Simon <[hidden email]> ---
Yes, context menu is obviously broken. I tested all kinds of things but
for unknown reasons never once did a right click. This code is also
heavily duplicated between table and icon view, I am thus refactoring
while fixing.

On 08/03/17 20:21, Maik Qualmann wrote:

> Simon,
>
> I think we do not need that much. A simple "use first image of grouped or use
> all images" for all actions should be enough. Let's see what Gilles says when
> he has time. I have also found some errors on the right mouse button context
> menu:
>
> - Assign color label to a group, the dialog asks for each image
> - Assign or remove tags no longer works
> - Assign rating no longer works
> - Assign flags no longer works
>
> Maik
>

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

Simon <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #104362|0                           |1
        is obsolete|                            |

--- Comment #4 from Simon <[hidden email]> ---
Created attachment 104490
  --> https://bugs.kde.org/attachment.cgi?id=104490&action=edit
Patch to customize grouping behaviour - 2

This is an actualized patch squashing some more duplicated code and fixing the
issues mentioned by Maik.

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #5 from Simon <[hidden email]> ---
Created attachment 104632
  --> https://bugs.kde.org/attachment.cgi?id=104632&action=edit
Deduplicate code between icon and table view

This is the part of the patch that is not actually directly related to
grouping. I just needed this to get consistent behaviour between the two views
and to get rid of duplicate code. In my opinion it is cleaner and much less
error prone. It probably makes sense if you first look at this and when this is
committed we can look at the grouping stuff.

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #6 from [hidden email] ---
Yes good improvement in source code.

Can you reproduce the icon/widget overlays on icon view, when you enable all
options from setup and when you change the icon size from status bar ?

Also turn on Hdpi support for icons view (thumb size > 256)...

Gilles

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #7 from [hidden email] ---
Note : it's voluntary to not include "Deduplicate" patch in "Customize" patch ?

Do you want to apply "Deduplicate" patch quickly in git/master ?

Gilles

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #8 from [hidden email] ---
[gilles@localhost core]$ patch -p1 < customizeGrouping-2.patch
patching file app/items/digikamimageview.cpp
patching file app/items/digikamimageview.h
patching file app/items/imagecategorizedview.cpp
patching file app/items/imagecategorizedview.h
patching file app/main/digikamapp.cpp
patching file app/utils/contextmenuhelper.cpp
patching file app/views/digikamview.cpp
patching file app/views/digikamview.h
patching file app/views/tableview/tableview.cpp
patching file app/views/tableview/tableview.h
patching file app/views/tableview/tableview_model.cpp
patching file app/views/tableview/tableview_model.h
patching file libs/models/imagefiltermodel.cpp
patching file libs/settings/applicationsettings.cpp
patching file libs/settings/applicationsettings.h
Hunk #2 succeeded at 463 (offset 3 lines).
patching file libs/settings/applicationsettings_miscs.cpp
Hunk #2 succeeded at 288 (offset 11 lines).
patching file libs/settings/applicationsettings_p.cpp
Hunk #3 succeeded at 236 (offset 1 line).
Hunk #4 succeeded at 366 (offset 2 lines).
patching file libs/settings/applicationsettings_p.h
Hunk #3 succeeded at 285 (offset 1 line).
patching file utilities/setup/setupmisc.cpp
patching file utilities/setup/setupmisc.h

[gilles@localhost core]$ patch -p1 < viewRefactor.patch
patching file app/items/digikamimageview.cpp
Reversed (or previously applied) patch detected!  Assume -R? [n]

Why ?

Gilles

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #9 from Maik Qualmann <[hidden email]> ---
Simon,

Short test here, cut, copy, paste, delete to the trash no longer works.

Maik

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #10 from Maik Qualmann <[hidden email]> ---
I have patch 104632 tested.

Maik

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #104490|0                           |1
        is obsolete|                            |

--- Comment #11 from [hidden email] ---
Created attachment 104633
  --> https://bugs.kde.org/attachment.cgi?id=104633&action=edit
Customize grouping behavior patch version 3

Updated patch to compile with git/master.

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #12 from [hidden email] ---
Simon,

With Customize patch applied, i can see this on the console :

digikam.general: Cancel Main Thread
QObject::connect: No such slot Digikam::DigikamView::deleteSelected() in
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2594
QObject::connect: No such signal
Digikam::ContextMenuHelper::signalslotCreateGroupByTime() in
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2655
QObject::connect: No such signal
Digikam::ContextMenuHelper::signalslotCreateGroupByFilename() in
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2658
QObject::connect: No such slot Digikam::DigikamView::deleteSelected() in
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2594
QObject::connect: No such signal
Digikam::ContextMenuHelper::signalslotCreateGroupByTime() in
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2655
QObject::connect: No such signal
Digikam::ContextMenuHelper::signalslotCreateGroupByFilename() in
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2658
digikam.geoiface: ----

The grouping behavior do not generate frame around thumb anymore...

Gilles

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

Simon <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #104632|0                           |1
        is obsolete|                            |

--- Comment #13 from Simon <[hidden email]> ---
Created attachment 104635
  --> https://bugs.kde.org/attachment.cgi?id=104635&action=edit
Deduplicate code between icon and table view (V2)

So the idea of the two patches is just to split this into smaller changes that
are easier to review. If the refactor has problems, the rest won't work either.

There were some typos and C&P errors in the context menu, the issues you (Maik
& Gilles) reported should be fixed now.

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

Simon <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #104633|0                           |1
        is obsolete|                            |

--- Comment #14 from Simon <[hidden email]> ---
Created attachment 104636
  --> https://bugs.kde.org/attachment.cgi?id=104636&action=edit
Customize grouping behavior patch version 4

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #15 from [hidden email] ---
"Customize" patch version 4 do not compile. I has fixed this problem in version
3 :

/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp: In member
function ‘QList<QAction*> Digikam::TableView::getExtraGroupingActions()’:
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:301:78: error: no
matching function for call to ‘QAction::QAction(QString)’
     QAction* const actionHideGrouped = new QAction(i18n("Hide grouped
items"));
                                                                              ^
In file included from /usr/lib64/qt5/include/QtWidgets/qmenu.h:40:0,
                 from /usr/lib64/qt5/include/QtWidgets/QMenu:1,
                 from
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:31:
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note: candidate:
QAction::QAction(QActionPrivate&, QObject*)
     QAction(QActionPrivate &dd, QObject *parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note:   candidate expects 2
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note: candidate:
QAction::QAction(const QIcon&, const QString&, QObject*)
     QAction(const QIcon &icon, const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note:   candidate expects 3
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note: candidate:
QAction::QAction(const QString&, QObject*)
     QAction(const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note:   candidate expects 2
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note: candidate:
QAction::QAction(QObject*)
     explicit QAction(QObject* parent);
              ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note:   no known conversion
for argument 1 from ‘QString’ to ‘QObject*’
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:311:78: error: no
matching function for call to ‘QAction::QAction(QString)’
     QAction* const actionIgnoreGrouping = new QAction(i18n("Ignore
grouping"));
                                                                              ^
In file included from /usr/lib64/qt5/include/QtWidgets/qmenu.h:40:0,
                 from /usr/lib64/qt5/include/QtWidgets/QMenu:1,
                 from
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:31:
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note: candidate:
QAction::QAction(QActionPrivate&, QObject*)
     QAction(QActionPrivate &dd, QObject *parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note:   candidate expects 2
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note: candidate:
QAction::QAction(const QIcon&, const QString&, QObject*)
     QAction(const QIcon &icon, const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note:   candidate expects 3
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note: candidate:
QAction::QAction(const QString&, QObject*)
     QAction(const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note:   candidate expects 2
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note: candidate:
QAction::QAction(QObject*)
     explicit QAction(QObject* parent);
              ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note:   no known conversion
for argument 1 from ‘QString’ to ‘QObject*’
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:321:82: error: no
matching function for call to ‘QAction::QAction(QString)’
     QAction* const actionShowSubItems = new QAction(i18n("Show grouping in
tree"));
                                                                               
  ^
In file included from /usr/lib64/qt5/include/QtWidgets/qmenu.h:40:0,
                 from /usr/lib64/qt5/include/QtWidgets/QMenu:1,
                 from
/home/gilles/Devel/5.x/core/app/views/tableview/tableview.cpp:31:
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note: candidate:
QAction::QAction(QActionPrivate&, QObject*)
     QAction(QActionPrivate &dd, QObject *parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:174:5: note:   candidate expects 2
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note: candidate:
QAction::QAction(const QIcon&, const QString&, QObject*)
     QAction(const QIcon &icon, const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:89:5: note:   candidate expects 3
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note: candidate:
QAction::QAction(const QString&, QObject*)
     QAction(const QString &text, QObject* parent);
     ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:88:5: note:   candidate expects 2
arguments, 1 provided
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note: candidate:
QAction::QAction(QObject*)
     explicit QAction(QObject* parent);
              ^
/usr/lib64/qt5/include/QtWidgets/qaction.h:87:14: note:   no known conversion
for argument 1 from ‘QString’ to ‘QObject*’
core/app/CMakeFiles/digikamgui_src.dir/build.make:1177: recipe for target
'core/app/CMakeFiles/digikamgui_src.dir/views/tableview/tableview.cpp.o' failed
make[2]: ***
[core/app/CMakeFiles/digikamgui_src.dir/views/tableview/tableview.cpp.o] Error
1
make[2]: *** Waiting for unfinished jobs....

QAction need a parent in constructor (qt 5.6.2 under Mageia 6)

Gilles

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #16 from [hidden email] ---
This warning need also to be corrected :

[ 90%] Building CXX object
core/app/CMakeFiles/digikamgui_src.dir/digikamgui_src_automoc.cpp.o
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp: In member function ‘bool
Digikam::DigikamView::needGroupResolving(Digikam::ApplicationSettings::OperationType,
bool) const’:
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2469:12: warning:
enumeration value ‘WelcomePageMode’ not handled in switch [-Wswitch]
     switch (viewMode())
            ^
/home/gilles/Devel/5.x/core/app/views/digikamview.cpp:2469:12: warning:
enumeration value ‘TrashViewMode’ not handled in switch [-Wswitch]

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #104636|0                           |1
        is obsolete|                            |

--- Comment #17 from [hidden email] ---
Created attachment 104641
  --> https://bugs.kde.org/attachment.cgi?id=104641&action=edit
Customize grouping behavior patch version 5

This version of patch fix 2 last problems that i report here...

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377197

--- Comment #18 from Simon <[hidden email]> ---
I intentionally removed "this" from these kind of calls:

     QAction* const actionIgnoreGrouping = new QAction(i18n("Ignore grouping"),
this);

The reason is that they will be children of DigikamView not of TableView.
According to QT5 specifications (http://doc.qt.io/qt-5/qaction.html#QAction) a
missing parent object should be acceptable. For me it compiles just fine, I
don't understand why it doesn't for you.

--
You are receiving this mail because:
You are the assignee for the bug.