vimiv-qt
vimiv-qt copied to clipboard
Add option to copy names of all marked images
At present time, one can copy the name of the currently selected image using :copy-name. This PR adds a way to also copy the names of all marked images.
Changes
- Flag
--markwas added to the command:copy-namewhich makes the command copy all marked images instead of the currently selected one. - Keybindings
ymA,ymY,yma, andymywere added as correspondent to the default copy bindings, but acting on the marked images.
Questions
I did not add this flag to :copy-image. Technically it should be possible, but it is a bit more involved; The only way how multiple files can be added to the clipboard is by constructing a QMimeData using the .setUrls setter. This however requires local paths and not a QPixmap directly. This is an issue if we would like to scale the marked images first, as we would have to store them in temporary files.
I can certainly add that if requested (i.g for consistency-reasons), but personally I have never missed that functionality.
Requires #705
Thanks! I definitely like this overall, but wonder why we would not just support paths instead? This would probably break backward compatibility, unless we add some workaround, but should be the straightforward fix using the existing logic of "expansion"?
I.e., something along (docstring not updated):
def copy_name(paths: Iterable[str], abspath: bool = False, primary: bool = False) -> None:
"""Copy name of current path to system clipboard.
**syntax:** ``:copy-name [--abspath] [--primary]``
optional arguments:
* ``--abspath``: Copy absolute path instead of basename.
* ``--primary``: Copy to primary selection.
"""
clipboard = QGuiApplication.clipboard()
mode = QClipboard.Mode.Selection if primary else QClipboard.Mode.Clipboard
text = " ".join(path if abspath else os.path.basename(path) for path in paths)
clipboard.setText(text, mode=mode)
We could then use things like:
:copy-name %
:copy-name %m --abspath
:copy-name * --primary
and so forth.
I am still undecided on copying images. Consistency would be great, the additional effort not. I'll try to take a more detailed look into the QMimeData stuff :blush:
Merry Christmas, and I hope you are doing good! 🎄
This is a great idea. I have implemented these changes in ad6e748f1078be0b2ed0f50d398f4ab331c04fe9.
Concerning backward compatibility. I think the best is when we make the positional argument optional for now and make it required from v1.0.0. on. A warning indicates to the user that the old way of calling is deprecated. What do you think?
I had a quick look into how making the argument optional. It is a bit more involved than I hoped due to the way how commands are registered and parsed by argparser (c.f. api/commands::_get_kwargs and related).
I would leave copy-image unchainged for now as there are more relevant things to do. Though, we could create an issue to remember it.
Wow, I completely missed this one although I did somewhat keep track of issues, sorry! The PRs are (rightfully) so spammed with the dependabot updates, that this one must have slipped ...
I am doing great in general, thanks :blush: how about you? Did you have a good start into 2024?
I agree that having it optional for now would be great, and I can see why this would be a hassle ... I also agree with keeping copy-image as is for now and creating an issue once this PR is merged to not forget it.