Add Copy to Clipboard for VersionInfo dialog

[r3yc0n1c]

Your checklist for this pull request

• I've read the guidelines for contributing to this repository
• I made sure to follow the project's coding style
• I've updated the documentation with the relevant information (if needed)

Detailed description

Adds a Copy, Close button in the VersionInfo Dialog to copy the formatted info to Clipboard.
Also adds translations support.

Test plan (required)

• Click on the "Version Info" button in the Dashboard.
• Click on "Copy" to copy the formatted version details
• Try pasting it somewhere
• Click on "Close" to close the dialog

Closing issues

Closes #3103

Screenshots

[karliss]

For this window I would prefer if selection and copying was functioning normally. No need to add a separate button, otherwise you could end up adding a copy button in every possible UI screen which will be cluttered a mess.

In my opinion there are few rare exceptions where dedicated copy button is acceptable and even preferred, but this is not one of them.

Allowing multiline selection should be trivial. Not sure about copying part, but that should also be solvable.

[wargio]

the issue itself had a copy all request.

[r3yc0n1c]

I get your point @karliss. This issue was a request for a Copy All function but we can definately go for the trivial selection in a new issue. Shall I create one?

[karliss]

First of all feature request by random user shouldn't be taken as truth for what the best solution to their problem is, considering rest of program is.

And even if you consider what that issue author said for me it doesn't seem obvious that custom "copy all" button is what he asked.

Personally i'd prefer a less fancy rendering, but providing for select-all and copy-all, like the extra details region of the About window does

The way I interpret it "less fancy rendering" means non broken selection/multiline copy which functions just like you would expect multiline selection to work. The about window he mentioned as preferred solution doesn't even have "copy-all". It simply has regular label with selectable text. And all the context menu actions like "Select all" and "Copy" are the default actions available to any text field with same configuration.

Also rest of the issue describes the problem as having selection behavior lacking ability to select multiple lines and copying them.

1. There is no context menu
2. the grids pretend to select full row, but when a hotkey is used, only a single column value is copied, not the whole selection.
3. No multi-row selection to copy 'all of them'
4. No multi-column selection either

I have no idea where did you both saw that it requested "Copy all" button.

[karliss]

Note enabling multiline selection isn't any complicated than what's already written in this PR. It's a matter of setting appropriate QTReeWidget::selectionMode, you can even do it in the UI file.

As for copying you can do something like this:

auto action = ui->rightTreeWidget->addAction("Copy", QKeySequence::Copy, this, [this]() {
    CopyWidgetSelection(leftTreeWidget);
});
action->setShortcutContext(Qt::ShortcutContext::WidgetWithChildrenShortcut);

CopyTreeWidgetSelection(QTreeWidget *treeWidget) {
    // almost the same code you already have, but for single treeWidget
    // ... 
    while (*itl) {
        if ((*itl)->isSelected()) {
            // your existing code for preparing copy content
        }
    }
    // no need to show a popup telling, that Ctrl+C isn't broken
    // ...
}

That way there is no need for additional header inserted in copied text to distinguish between left and right side, code for left and right side gets reused, no need for additional button panel. No need for additional popup confirming that copying succeeded since Ctrl+C (or equivalent platform specific shortcut) working everywhere is the expected normal behavior.

[wargio]

First of all feature request by random user shouldn't be taken as truth for what the best solution to their problem is, considering rest of program is.

And even if you consider what that issue author said for me it doesn't seem obvious that custom "copy all" button is what he asked.

That is true, my bad.

@r3yc0n1c please follow the directives that @karliss has proposed, it will vastly improve the feature!

[r3yc0n1c]

Got it... I'll update this ASAP!
Thanks!

[r3yc0n1c]

hi, I've been experimenting with different approaches for this implementation. Here's the recent update: dev...r3yc0n1c:cutter:exp-copy-feat-versioninfo

I'm facing - only 1st item is selected/sometimes 'QAction::event: Ambiguous shortcut overload: Ctrl+C' error... could you please review this?

[karliss]

A couple of potential issues all from the fact that you are creating a new action each time a context menu is opened:

• how is the action with shortcut supposed to work if you only create it when opening a context menu
• you are creating new action associated with VersionInfo Dialog each time context menu is opened. That can not only bloat memory usage, but is also probably one of reasons why you are getting warning about ambiguous shortcut. This is C++ you have to think about lifetime of objects. Qt introduces it's own mechanisms for managing this, but it doesn't change the fact that you have to think about object lifetimes

The actions should be created during initialization of window. That way the keyboard shortcuts can work before context menu is opened. Afterwards if necessary you can add the existing action to a context menu if necessary. Although in this case you could set the contextMenuPolicy to Qt::ActionsContextMenu, thus avoiding code for manually filling custom context menu. One more aspect to this approach is that you would have to create a separate but similar action for each treewidget, also make sure parent of action is the treewidget not the dialog. That will avoid the need for, in my opinion hacky, check of which tree widget has focus, it's also necessary for Qt::ActionsContextMenu to work.

In general (but not this specific situation) you can create the actions for context menu in the contextMenuEvent callback, and a lot of places in Cutter do that, but there are a few things you need to take into. If you create the actions for context menu in the callback, they should be associated with the temporary menu object instead of the dialog. That way they will be destroyed together with temporary menu, instead of accumulating each time you open the context menu.
Doing it that way is not very suitable for actions with shortcuts. This approach is more suitable for dynamic actions which don't have a shortcut and the amount of which depends on the thing you clicked.

[r3yc0n1c]

Hello @karliss I've made the changes following your heads-up, and it's working fine locally. Could you please check this once - 219f27d

Thanks!

[karliss]

@r3yc0n1c In case you missed it, the comment for column constants is still opened.

[r3yc0n1c]

Hi @karliss @XVilka , could you please review the changes?

[XVilka]

I tested it on Windows, and it works. But once I close the Version window and open it again, the selection is still active. I think it should be dropped/reset upon window closure, so once you open it again, nothing is selected.

[karliss]

No more new objections from me. Would be good to fix the thing XVilka said before merging.

[XVilka]

Unnecessary change

[karliss]

How is this considered resolved?

[XVilka]

I cannot test right now, but from the code perspective I don't have any objections.

[karliss]

How is this considered resolved?