Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add run command functionality #636

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fkubicek
Copy link
Contributor

This PR adds an action that runs a user-defined command with the path to the currently opened image as an argument. Solves #579

  • A new action "Run Command" is added with keyboard shortcut setting, and a button in the "Tools" menu
  • The command can be specified in settings. The path is specified as a wildcard %QVIEW_PATH% anywhere in the command (even multiple times, if the user wishes)
  • A new tab in settings, "File Operations" is added. It contains the above setting and file delete settings that were moved from the Misc. tab. Adding a new tab was not my first option. Originally, I wanted to put this in the Misc. section, but it was really cluttered and big - it altered the size of the entire settings windows. There was already a relatively large number of setting in the Misc. tab, so I think this is the best option.
  • Code that executes command (mainwindow.cpp function) is fully portable (only uses Qt API)
  • Bonus: tab order in settings was fixed

Tested on Win 10 and Kubuntu 22.4

@jurplel jurplel self-requested a review October 7, 2023 19:13
@fkubicek fkubicek force-pushed the add-run-command-functionality branch from 3bbe2b2 to d77cfbd Compare October 15, 2023 12:33
@fkubicek
Copy link
Contributor Author

@jurplel I fixed the compile error. I was using some methods from newer QT API and didn't realize that older versions were supported too.

@vassudanagunta
Copy link

First, thanks for taking a hopefully successful stab at this!

Second, some questions. Maybe I should figure out how to build qView (i.e. with these changes, on macOS), but maybe it would be easier, and helpful for others, if you could tell us:

  • Does this support a single custom run command, or can you add as many as you want? Could you add a screenshot of the new settings tab to this PR?

  • Does it support invocation of any executable? For example, one macOS one would want to be able to invoke shell commands (e.g. bash), OS registered apps, and AppleScripts.

@fkubicek fkubicek force-pushed the add-run-command-functionality branch from d77cfbd to df8e7d7 Compare October 15, 2023 14:35
@fkubicek
Copy link
Contributor Author

fkubicek commented Oct 15, 2023

Hi, good idea 🙂 Here is a screenshot from Windows, this setup launches my Python script with the currently opened image path as an argument:
image
It only supports one command, I would rather keep this feature minimalistic and simple. I think that is more in line with qView style.

Btw, the addition of the "File Operations" tab can be easily reversed, it's in a separate commit.

To answer your second question, well, I'm not sure, because I don't have a Mac anywhere near me, but on Kubuntu for example, I have tested rm %QVIEW_PATH% and it worked just fine. The command must be simple, just program name and arguments, because I used QT API, which takes program name and arg list separately. So I'm not really sure what the answer is, it depends on what QT can do, but it worked fine for me for all of my usecases. I tried mainly executables and scripts on Windows, eg. explorer %QVIEW_PATH, and a couple scripts and commands on Kubuntu.

Edit:
I realized that rm is not a shell built-in command, but I can't really figure out which of the buit-in commands would be practical to test this. But I think purely shell built-ins wouldn't work anyway. I tried things like kill, ls, echo etc. but I think they are all executables. What doesn't work is stuff like this: pwd && ls. This is interpreted as pwd with args && and ls.

I would argue that this is fine, because it covers a lot of the use cases like opening the file in a different program. And for anything more complex, you can just write a script and execute that with the filename as an argument. That way, literally anything is possible.

Here's a Kubuntu screenshot as a bonus 😀
image

@begabba
Copy link

begabba commented Oct 15, 2023

Well done, @fkubicek
However, personally, I would love for this feature to support multiple commands with unique keybindings.
This would allow me, for example, to quickly sort images by moving them to different folders with a single key press. This would greatly increase the usefulness of qView in my opinion.
I'm not really sure on how the UI would look but I'll start by brushing up my Qt skills.

@fkubicek
Copy link
Contributor Author

Thanks :)
I'm not sure how supporting multiple commands with unique keybindings would look like either. I don't think that a "variable number of keyboard shortcuts" feature could be added easily, and the other option is just copy-pasting what I have done a set number of times, maybe ten times? But that would be quite ugly IMO.

So I would prefer to keep it as it is for now. But you can still do what you want, just write a script that does different things. I tried it in Python, it took just a few minutes:

import sys
import shutil
from pathlib import Path    
from tkinter import *

DESTINATIONS = {
    '1': r"C:\Users\Fikusekk\Desktop",
    '2': r"C:\Users\Fikusekk\Downloads"
    }

image_path = Path(sys.argv[1])

def keydown(e):
    destination = Path(DESTINATIONS[e.char])
    shutil.copyfile(image_path, destination.joinpath(image_path.name))
    sys.exit(0)


root = Tk()
frame = Frame(root, width=100, height=100)

frame.bind("<KeyPress>", keydown)
frame.pack()
frame.focus_set()
root.mainloop()

If you call this from qView like this py "C:\Users\Fikusekk\Desktop\copyto.py" %QVIEW_PATH%, it's just one key pressed to open the Python window and the next keypress (1 or 2 key) copies your picture where you want.

My point is, since you can launch a script, you can do pretty much anything you want from that poin on.

@vassudanagunta
Copy link

vassudanagunta commented Oct 16, 2023

@fkubicek

I would argue that this is fine, because it covers a lot of the use cases like opening the file in a different program. And for anything more complex, you can just write a script and execute that with the filename as an argument. That way, literally anything is possible.

Agreed.

It only supports one command, I would rather keep this feature minimalistic and simple. I think that is more in line with qView style.

To be frank, that's like justifying only supporting one custom keyboard shortcut to keep things minimalist and simple. But looking at the Shortcuts tab of qView Preferences reveals the real reason for this limitation: qView has a fixed table of key bindings. I guess I'm used to apps that support an open list of key bindings, such as my IDE. Or IINA, the current best video player on macOS:

Screen Shot 2023-10-15 at 7 40 04 PM

IINA's key bindings can be mapped to IINA's UI controls, or to any command supported by the underlying video player, mpv. Display Raw Values reveals the underlying commands and arguments:

Screen Shot 2023-10-15 at 7 40 21 PM

So given the current way qView does key bindings, @fkubicek, it makes sense that you only support the one.

the other option is just copy-pasting what I have done a set number of times, maybe ten times? But that would be quite ugly IMO.

Definitely ugly. The right way is like what IINA does above. But not expecting you to tackle that!

One is better than none! Infinitely better if 1 ÷ 0 = ∞! Much appreciated. I hope the code changes are acceptable to @jurplel. 🤞🏾

@fkubicek
Copy link
Contributor Author

Thanks for feedback, I'm glad you like the the way it is now 😁 Let's wait for the final review.

@jurplel
Copy link
Owner

jurplel commented Oct 20, 2023

Great discussion here and great work. Once I find time to make a release again, I would like to adapt it to be closer to @vassudanagunta's suggestion. It would be much more useful for sure. This would also likely come with redesigning the settings page, similar to @jdpurcell's mockup with a sidebar. The settings dialog is already so crowded, it needs an overhaul to get more functionality. I also love IINA and the way that they do things.

@iamvanja
Copy link

I really like Xee's approach.

Screenshot 2023-11-27 at 22 41 39

Side panel opens and it is possible to use GUI to copy/move. It is also possible to press CMD + number without even having the side panel active.

I love that this copy/move functionality, in any shape or form, is coming to qView! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants