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

[bugzilla_noreply]

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

--- Comment #19 from [hidden email] ---
Qt 5.6 LTS need certainly an explicit parent, or null. You must pass a parent
to QAction constructor.

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 #20 from Simon <[hidden email]> ---
Created attachment 104648
  --> https://bugs.kde.org/attachment.cgi?id=104648&action=edit
Deduplicate code between icon and table view (V3)

Ok, thanks for fixing. I wasn't aware of a QT LTS version we should be
compatible with.

Also about the split patch: I would indeed like to get the refactoring part
merged independently, as I assume the grouping part will need longer discussion
(?) and another issue I was looking at recently would be easier to fix with
this in place already.

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

[bugzilla_noreply]

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

Vincent Tassy <[hidden email]> changed:

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

--- Comment #21 from Vincent Tassy <[hidden email]> ---
Will this patch also fix the current behaviour of the Slideshow (that shows all
images in the groups instead of the visible one) ?

Or does something need to be changed in the Slideshow or Selection code ?

--
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 #22 from Maik Qualmann <[hidden email]> ---
Simon,

patch 104648 does not compile...

/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:
In member function ‘QList<QAction*>
Digikam::TableView::getExtraGroupingActions() const’:
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:297:84:
error: invalid conversion from ‘const QObject*’ to ‘QObject*’ [-fpermissive]
     QAction* const actionHideGrouped = new QAction(i18n("Hide grouped items"),
this);
                                                                               
    ^
In file included from /usr/include/qt5/QtWidgets/qmenu.h:46:0,
                 from /usr/include/qt5/QtWidgets/QMenu:1,
                 from
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:31:
/usr/include/qt5/QtWidgets/qaction.h:94:14: note:   initializing argument 2 of
‘QAction::QAction(const QString&, QObject*)’
     explicit QAction(const QString &text, QObject *parent = nullptr);
              ^~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:307:84:
error: invalid conversion from ‘const QObject*’ to ‘QObject*’ [-fpermissive]
     QAction* const actionIgnoreGrouping = new QAction(i18n("Ignore grouping"),
this);
                                                                               
    ^
In file included from /usr/include/qt5/QtWidgets/qmenu.h:46:0,
                 from /usr/include/qt5/QtWidgets/QMenu:1,
                 from
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:31:
/usr/include/qt5/QtWidgets/qaction.h:94:14: note:   initializing argument 2 of
‘QAction::QAction(const QString&, QObject*)’
     explicit QAction(const QString &text, QObject *parent = nullptr);
              ^~~~~~~
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:317:88:
error: invalid conversion from ‘const QObject*’ to ‘QObject*’ [-fpermissive]
     QAction* const actionShowSubItems = new QAction(i18n("Show grouping in
tree"), this);
                                                                               
        ^
In file included from /usr/include/qt5/QtWidgets/qmenu.h:46:0,
                 from /usr/include/qt5/QtWidgets/QMenu:1,
                 from
/home/maik/Devel/digikam-software-compilation/core/app/views/tableview/tableview.cpp:31:
/usr/include/qt5/QtWidgets/qaction.h:94:14: note:   initializing argument 2 of
‘QAction::QAction(const QString&, QObject*)’
     explicit QAction(const QString &text, QObject *parent = nullptr);
              ^~~~~~~
make[2]: *** [core/app/CMakeFiles/digikamgui_src.dir/build.make:1178:
core/app/CMakeFiles/digikamgui_src.dir/views/tableview/tableview.cpp.o] Fehler
1

--
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 #104635|0                           |1
        is obsolete|                            |
 Attachment #104648|0                           |1
        is obsolete|                            |

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

@Maik Sorry for the inconvenience, even C&P should be tested...

@Vincent Yes, the current state also addresses the behaviour of groups
regarding slideshow. You will be able to choose whether to include all or only
the first (visible) image.

--
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 #24 from Maik Qualmann <[hidden email]> ---
Simon,

"Move To Album..." does not work.

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 #104675|0                           |1
        is obsolete|                            |

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

Fixed moving to album - thanks 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 #104641|0                           |1
        is obsolete|                            |

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

--
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 #104716|0                           |1
        is obsolete|                            |

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

Update to 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

Simon <[hidden email]> changed:

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

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

--
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 #104827|0                           |1
        is obsolete|                            |

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

I would like to get this done, as there were some reports from users who had
problems with the "new" grouping behaviour. Also some other things I wanted to
fix depend partially on this.

Can I commit the refactor part (@Maik)?

The grouping part probably needs more attention.

--
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 #30 from Maik Qualmann <[hidden email]> ---
I have not found any problems with the V8 patch. But I think we do not need to
distinguish between BQM, light table, renaming, etc. An option for all cases
(yes, no, ask) should be enough. Gilles, Mario what is your point of view?

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

Mario Frank <[hidden email]> changed:

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

--- Comment #31 from Mario Frank <[hidden email]> ---
(In reply to Maik Qualmann from comment #30)
> I have not found any problems with the V8 patch. But I think we do not need
> to distinguish between BQM, light table, renaming, etc. An option for all
> cases (yes, no, ask) should be enough. Gilles, Mario what is your point of
> view?
>
> Maik

I agree. Keep it simple if there is no really good reason. And I currently see
none.

--
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 #32 from Simon <[hidden email]> ---
Originally the request originated around tagging. After the change users
started complaining mostly about slideshow and lighttable. And for my personal
workflow it absolutely makes sense to do operations like moving and tagging
images together. However with slideshow and lighttable, it doesn't really. For
bqm it depends on what you want to do.
What I took out of all the responses is, that there is a multitude of different
workflows and I will never be able to think of each and every one. That's why I
tried to make it as customizable as possible.

--
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 #105016|0                           |1
        is obsolete|                            |

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

I updated this with information about the operation in the dialog, streamlined
the setup page creation and added some documentation.

--
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 #34 from Simon <[hidden email]> ---
Created attachment 105389
  --> https://bugs.kde.org/attachment.cgi?id=105389&action=edit
Customize grouping behavior patch version 10

Version 10 fixes compiler warnings.

There was no more problems reported, the refactoring has been met positively
and I need it to do further improvements - so unless there is opposition, I
will push the non-grouping related changes (104828) to master soon.

Regarding the grouping and whether to have separate options for different
operations or just one option: Can you reply to my earlier statement explaining
why I think it is worth having separate options please.
Even if the answer is, that you still want the simpler version, that is better
than just letting this hang around. I will be disappointed (and run the full
version locally, as I already got used to it), but one option to control all is
still better than no option.

--
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 #35 from Maik Qualmann <[hidden email]> ---
Created attachment 105393
  --> https://bugs.kde.org/attachment.cgi?id=105393&action=edit
grouping.png

As I said, I think it would not be necessary if Gilles did not mind, I would
also agree. But see screenshot, the first line has no text. Everything stands
on ask, but the kipi-Plugins take only the first image of the group, no dialog.

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 #36 from Maik Qualmann <[hidden email]> ---
Ignore the thing with the empty text line, I have not seen right. But the other
thing does not work.

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 #37 from [hidden email] ---
Maik,

No objection from me. I just follow the file only and let's you to test Simon
patches.

Note : kipi interface do not support yet grouped items. If i remember, there is
a generic file in bugzilla about this topic.

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 #38 from Simon <[hidden email]> ---
I just had a look and couldn't even figure out how the selection makes it to
the kipi plugins. Apparently I just added the Kipi member to the enum as I
considered it relevant and then never followed through actually implementing
it. If you can give me a pointer for how this currently works, I might be able
to work around this limitation, otherwise I will remove kipi from this patch.

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