Allow reading stdin/pipes as images

[ThinkChaos]

Requested feature

Support opening pipes, or at least stdin.

Use case

I want to use drawing when I take a screen shot, basically like screenshot | drawing -, or drawing <(screenshot).
This avoids needing to store a temporary file, and makes the flow simpler: only explicitly saved screen shots are ever stored. This happens a fair amount because my screenshot commands includes a step where I select a part of the screen with my mouse, which I often decide I want to redo.

How to test

You can replace screenshot in the above examples with cat /path/to/image.

Implementation idea

I think the best would be to detect if a given file is a pipe, read it and try to create a DrImage with the bytes.
DrImage would need to be adapted to also support using bytes instead of a gfile, so no file watching and create a pixbuf from bytes with: GdkPixbuf.Pixbuf.new_from_bytes.

In the case the read bytes are not an image you'll need to display an error window, but that could also help fix the TODO in _get_valid_file: remove the utilities_gfile_is_image check and in the DrImage the pixbuf will give you an exception you display.

[maoschanz]

that's pretty niche 🤔

people may prefer pressing ctrl (or whatever they set as a keyboard shortcut) when doing their screenshot, so it goes in the clipboard instead of a new file, and then you can right-click on the app icon and choose "edit image in the clipboard" (or open it from the CLI with -c if you really want bash scripts)

that could also help fix the TODO in _get_valid_file

no, there is a comment in utilities_gfile_is_image explaining what happens here:

• there is already a second check in image.py, raising a InvalidFileFormatException if necessary, and displaying the related message in the GUI. But those are situations where the file is an actual image. The user's GdkPixbuf lib is not able to load it for some reason, i warn them about it but it's none of my concern.
• the first check in main.py is here to prevent opening empty-tabs-with-an-error-message for completely absurd files. E.g. someone provides a list of PDFs: i don't even want to open tabs for them!
  • first because i don't want people to load any random bullshit with my code, as a safety concern
  • then because it's likely a mistake on their part, and if they press "save" they would replace their tax_declaration.pdf with a blank png
  • so i don't want to even create a DrImage: the message that _get_valid_file could display is 100% independent from the concept of a tab where i load an image file
  • Notice that the only MIME files declared in the .desktop file are images, so this case shouldn't happen aside of an erroneous CLI auto-completion: it's a mere "TODO" because only printing a message in the terminal is fine here in my opinion

you could remove that first check and things would still work fine, InvalidFileFormatException would be raised too if DrImage gets a pdf, i just think it would be inelegant to pass down such an absurd file as an argument in so many methods across so many classes

I think the best would be to detect if a given file is a pipe

stdin isn't an argument like the file names. In this line https://github.com/maoschanz/drawing/blob/master/src/main.py#L146 args[1] is a Gio.ApplicationCommandLine, and this object has a get_stdin() method

but it provides a stream, it's not easy to get the data from it, especially since gtk apps are unique instances: cli invocations often activate the existing instance and die right away. I don't know how to deal with that, but if it's feasible i will try

[ThinkChaos]

I tried with the clipboard but I use wayland and it's not working. If you want me to help debug that, feel free to give me a custom version to run. I subscribed to issue #377, so leaving a comment there is good enough for me to notice.

you could remove that first check and things would still work fine, InvalidFileFormatException would be raised too if DrImage gets a pdf

That was what I meant yes. I understand your concern about creating a useless tab. It was a quick suggestion so feel free to ignore.

stdin isn't an argument like the file names. In this line https://github.com/maoschanz/drawing/blob/master/src/main.py#L146 args[1] is a Gio.ApplicationCommandLine, and this object has a get_stdin() method

In general apps that use files handle - as meaning "open stdin".
So I was suggesting either following that pattern, or detecting if a path if a pipe:

fd=<(cat /path/to/whatever)
python -c 'import os, stat; assert stat.S_ISFIFO(os.stat("'$fd'").st_mode)'

# Note that stdin is a char device, not a FIFO/pipe:
python -c 'import os, stat; print(stat.S_ISCHR(os.stat("/dev/stdin").st_mode))'

In your loop over arguments in on_cli, you can use S_ISFIFO to know to handle the fpath differently.

EDIT: I should also mention I don't use a DE. That's why my screenshot keyboard shortcut runs a shell pipeline and doesn't match what you're used to.

[maoschanz]

In general apps that use files handle - as meaning "open stdin".
So I was suggesting either following that pattern, or detecting if a path is a pipe

this would be a great solution if i were developing from scratch, but here, reinventing the wheel with the os module would be a source of bugs and a bad coding practice: Gio has already parsed it for me, i don't even know the actual command line

i have a Gio.InputStream and i have to read it (asynchronously if i want to be clean, and it's always tricky to have scenarii with several threads), manage its errors, and close it. I said it's not easy compared to normal arguments, not that it's impossible and i have to parse command lines manually...

[ThinkChaos]

You don't need to parse the command line manually, just add an if in the loop on arguments:

if fpath == '-':
    print("Do something special here")

For stdin it'd be better to handle it synchronously: when running a shell pipeline, all processes run at the same time. So blocking on stdin is perfect since it pauses the program until all the previous ones had ran.
That's how all CLI utils work. I get that this app is a GUI, but if adding a feature like this I think it makes sense to follow this convention.

In my case, not blocking on stdin would mean that screenshot | drawing - would open drawing's windows before I'm able to take my screenshot.

[maoschanz]

it'd be better to handle it synchronously

i agree with the idea, but i'm not sure whether or not it's possible:

gtk apps are unique instances: cli invocations often activate the existing instance and die right away

i don't know if this singleton pattern works the intended way on something as "KISS" as nix without a DE, but it's the default setup for everyone

if the idea is feasible, its behavior has to consistently make sense for both "commands starting the instance" and "commands activating the previously started instance", and i'm afraid we will only know that after i try to implement it

[ThinkChaos]

I see. From my tests, the behaviour depends on whether it's the first window that's opened or not:

• First launch: on_cli is called before any UI shows up, so doing the sync read there should have the desired effect
• Other launches: on_cli is called in the main process, so no access to stdin from the other process, and obviously there already is existing UI.

Here's my thoughts on other/second launch just in case though (again I don't know much about GTK so I might be very wrong):
For second launch I think it depends on how much you can communicate between the second launch instance and the main instance. If you have full RPC access, then it should be possible to, in case of fpath == '-', have the singleton asks the new instance to read stdin and send it back over RPC. Having the new instance read stdin synchronously would be enough to have it block and thus work well in a shell pipeline.
Now I'm not sure how GTK calls on_cli but it should be easy enough to make the singleton instance waiting on stdin async:

1. RCP call to the new launch instance asking for stdin
2. Skip that fpath (just continue the loop)
3. It's now doing it's normal flow and asyunchronously "waiting" for a returning RPC call on a new method: on_read_stdin
4. on_read_stdin is called back with the data, proceed with opening it

Honestly for my use, only supporting stdin on first launch is good enough. So avoiding all that complexity and just sticking with a sync read(stdin) (or whatever the GTK stream equivalent is) in for arguments: if fpath == '-' is perfect.

Maybe adding a non-singleton/isolated launch mode would be a good middle ground, but I have no clue if that's easy with GTK.

[LamprosPitsillos]

I would also like this feature please, it would make it behave also a tool other then just a drawing app.
Thanks