stig
stig copied to clipboard
Add a command `fileopen` to open files in external programs
Thank you. I've had this on my todo list aswell.
-
Please squash your fixup commit so it is easier for me to review.
-
"Ugly hack to make the command behave as expected in the tui" doesn't tell me much. Can you explain what this hack does exactly and why it is needed?
-
It doesn't look like you need mixin.polling_frenzy. This is only needed if you make a request that changes state in transmission-daemon (e.g. stop a torrent) and you want to display that change ASAP in the TUI.
-
I haven't tried, but it looks like a failing command doesn't tell me what went wrong, and a command that only prints to stdout does nothing.
Maybe you can subclass something like TextIOWrapper[1] and delegate write() calls to self.info() for stdout and self.error() for stderr.
-
Once I'm happy with the PR, tests would be nice if you have the time.
[1] https://docs.python.org/3/library/io.html#io.TextIOWrapper
Hi, thanks,
- Fixed.
- When running
fopen mpv
in the tui with files selected the intuitively expected behaviour is that the files are opened withmpv
. This means that the two first arguments as the command is defined for the cli need to be filled in, sofopen mpv
should "alias" to ~fopen torrentid=x file=movie.mkv mpv
. Maybe it is better to have two base command classes? I am not super familiar with the structure of this code base yet so I don't know if that's possible or good. - Fixed.
- Yes -- my use case was opening files in
mpv
so I did not want the logging input instig
. I'll look into your suggestion. Perhaps it would also be good to have a--quiet
flag forfopen
, but then we'd need a way to separate it from arguments to passed through (e.g. one could callfopen --quiet mpv --fullscreen
and this would open the file fullscreened inmpv
while suppressing all output.) - Naturally -- is there documentation for writing tests? I'd need a mock
transmission
right?
-
I have some minor style complaints (read the next point first):
@.: Missing empty line above the method @.: Missing empty line above the method @.: Missing empty line above the method @.: Missing empty line below the method @.: Surplus empty line @.: Missing two empty lines above the class
-
It's been a long time since I've worked on this. After reading my own code and docs for a while, I think my plan for this was to make a stupid "run" command that just executes a command and logs the output. The "interactive" command should be able to supply the file/torrent path to "run". Something like this:
:bind --context file interactive "run --quiet mpv -f '{location}/{path}'" :bind --context torrent interactive "run --quiet mpv -f '{location}'"
This approach should drastically reduce the added code. The downside is that this doesn't work on the CLI. Is "stig fopen ..." important to you?
-
I agree that a --quiet/-q option would be best to ignore stdout. Maybe count the number of --quiet arguments and also silence stderr if >= 2.
Check out the "bind" command, which also accepts options and a command.
-
The tests were written in the Wild West. No docs or even a guide. You can look at existing tests for inspiration, but don't look too closely, I didn't really know what I was doing. :)