Is the annotation feature really necessary?

[guijan]

My 2 main grudges with it are that its interface is very clunky and that it's out of a screenshot program's scope.

The manual's example already shows some of that:

scrot -n "-f '/usr/share/fonts/TTF/DroidSans-Bold/40' -x 10 -y 20 -c 255,0,0,255 -t 'Hi'"

The entire argument to -n needs to be quoted so getopt_long() doesn't confuse it with one of its flags.
The user has to point scrot towards a font file instead of a font name because it doesn't use fontconfig to pick the font.
The syntax unnecessarily differs from other syntaxes used in scrot, such as the syntax of the -s flag or the -l flag. Really, all of those should just use the POSIX getsubopt() function instead.

I've actually needed an annotation feature in the past, but I used ImageMagick for it:

$ convert 2022-09-04-194335_956x536_scrot.png -pointsize 70 -fill Plum -stroke Gold -strokewidth 3 -gravity NorthEast -annotate +10+10 'test' out.png

ImageMagick's interface seems a little better, too, so I suggest removing annotations from scrot and leaving that to a different program. Would that be acceptable?

[brianredbeard]

Seems reasonable. Part of resurrection is trimming dead branches off of the tree.

[N-R-K]

Agreed. This is something more appropriate for an image editor instead of a screenshotting tool.

[ifohancroft]

I agree as well. It doesn't belong in scrot. The fact that removing it would also make the code smaller and better, that's a nice bonus.

P.S. Speaking of dead branches, what do you think about the thumbnail, the countdown, the command execution and the imlib2 script?

Here are my thoughts against them:

1. The thumbnail - I don't think it's scrot's job, and I don't know how useful that feature is. Most filemanagers create thumbnails of everything you've viewed, automatically, anyway. If you want to upload it somewhere, many sizes resize the images themselves, also if you need a specific resizing, you can use -e or dump the image to stdout and pipe it to pass it to something.

2. The countdown - You are only going to see it if you have opened the terminal and invoked scrot manually. If you invoke scrot from a script and passed it a delay, chances are you already have that delay saved in a variable somewhere. You are most likely to use that same variable for a sleep function after scrot, instead of using the countdown's output, or even the command's exit to know when it has finished.

3. The command execution (i.e. -e) - I actually love it and use it, but is it really scrot's job to do that? We have the file saving and the dumping to stdout for that.

4. The imlib2 script - again, is it scrot's job to do that? Shouldn't that be an image editor's job?

[guijan]

I agree as well. It doesn't belong in scrot. The fact that removing it would also make the code smaller and better, that's a nice bonus.

P.S. Speaking of dead branches, what do you think about the thumbnail, the countdown, the command execution and the imlib2 script?

1. The thumbnail

I'm indifferent to this one. It's probably out of scope, true. Being too aggressive about removing what is out of scope doesn't help either, though. For instance, scrot is definitely not an image encoder, but we don't limit ourselves to outputting bitmaps and leaving image encoding to another program.

2. The countdown

I would actually have scrot always countdown when a delay is used, the image isn't being output to stdout, and stdout is a TTY. That seems like an elegant solution to me.

Also, the current delay code suffers from drift. It calls sleep() in a loop, time passes between each sleep() call. It should use the POSIX setitimer() function instead which doesn't suffer from this issue.

3. The command execution (i.e. -e)

Yes. This feature actually interferes with capability limitation frameworks like FreeBSD's capsicum and OpenBSD's pledge() and unveil(). I intend to add support for those plus at least the Linux landlock and possibly its seccomp() too at some point.

"-e" isn't actually useful because shell pipelines exist, it's just code that shouldn't be there. The implementation also uses the system() function which should never be used.

I would instead have scrot always print the filename of the file it created unless the output file is stdout.

4. The imlib2 script - again, is it scrot's job to do that? Shouldn't that be an image editor's job?

I haven't even figured out how to use that one. imlib2 is badly documented and scrot's man page only says the option exists.

[ifohancroft]

1. Thumbnail 

I'm indifferent to this one. It's probably out of scope, true. Being too aggressive about removing what is out of scope doesn't help either, though. For instance, scrot is definitely not an image encoder, but we don't limit ourselves to outputting bitmaps and leaving image encoding to another program.

Fair enough.

Btw, this is a bit off topic, but speaking of image encoding - I think we can benefit from an option to specify what format you want the image in, in-case you don't specify a filename and just dump it to stdout for example.

2. The countdown 

I would actually have scrot always countdown when a delay is used, the image isn't being output to stdout, and stdout is a TTY. That seems like an elegant solution to me.

Interesting. Would you mind explaining your use case for the countdown? How/when do you find it useful? I'm not saying it is. I just didn't think it was used, so I'm curious how you use it.

3. The command execution (i.e. `-e`)

Yes. This feature actually interferes with capability limitation frameworks like FreeBSD's capsicum and OpenBSD's pledge() and unveil(). I intend to add support for those plus at least the Linux landlock and possibly its seccomp() too at some point.

"-e" isn't actually useful because shell pipelines exist, it's just code that shouldn't be there. The implementation also uses the system() function which should never be used.

I would instead have scrot always print the filename of the file it created unless the output file is stdout.

To be honest, I'm not sure how I feel about scrot outputting the filename. Usually you know what the filename is, either because you've set it, or if you haven't you know the default format, so I think it's a bit pointless implementing it.

If it does get implemented however, perhaps as an optional parameter instead of doing it by default?

Also, I haven't heard about capability limiting frameworks before. What's the idea there?

4. The imlib2 script - again, is it scrot's job to do that? Shouldn't that be an image editor's job?

I haven't even figured out how to use that one. imlib2 is badly documented and scrot's man page only says the option exists.

Honestly, same. I am guessing the idea is to make imlib2 do some processing on the image or something.

[guijan]

Interesting. Would you mind explaining your use case for the countdown?

I don't use it. I just think it's okay as a feature in a screenshot program, but it could use some better design. It's harmless to print a countdown when the program is interactive, so we could just skip some lines of code, remove the flag, and do it every time the program is interactive and there's a delay.

If it does get implemented however, perhaps as an optional parameter instead of doing it by default?

Maybe? I don't like sticking everything in an option. It's useful both interactively and uninteractively, you don't have to guess the filename, you get the exact filename back. If the user doesn't care about the filename, printing it is still harmless, so we could just do it unconditionally.

Also, I haven't heard about capability limiting frameworks before. What's the idea there?

You call a function early in the program to communicate to the kernel you will only ever make certain syscalls, and if the program fails this promise, it gets terminated. That way, if your program gets hacked during the course of its execution, there is a large set of tasks the hacked program is unable to do. For instance, scrot can promise it'll never make the syscalls related to networking before parsing args, and then somehow get hacked after that, and the hacked scrot will be unable to talk back to the hacker over the network.

You can also tell the kernel you will only ever access a directory or a specific file and only ever with certain permissions, and if the program fails this promise, functions like open() will return EACES or ENOENT. scrot in particular can easily promise it'll never ask for read permissions on anything. This way, a hacked scrot can't read sensitive data like SSH keys.

You can only ever make these capabilities stricter, you can never loosen them, and the kernel enforces them, they're a security mitigation. Many OSes these days have some way or another or doing this sort of thing, Linux in particular has a dozen of overlapping kernel features that do something like this, each implemented by one organization, and I'm aiming for the ones that are C functions.

Implementing this is much better if the program parses options as early as possible, which scrot doesn't do. You can make the capabilities much more limited if you design the program with them in mind, and the "-e" option is one useless feature that adds a lot of capabilities because the exec'd program can be any program. OpenBSD's pledge() is the only one that can set capabilities for forked programs separately, all the others need to postpone limiting their capabilities a lot or use a much broader set because "-e" can fork and exec.

[ifohancroft]

Honestly, I don't really like a couple of the ideas:

1. I don't like the idea of scrot always printing the countdown. I'd much rather it stay as it is, with an optional flag.
2. I don't like the idea of scrot sending the filename to stdout, I just find it inelegant and a bit werid and honestly, I'd much rather the -e flag stay, if the alternative is to always print the filename (or to be honest, even as an optional parameter to print the filename).

If do we those two, I plan on forking scrot so I can remove those two, and use that version.

I love the idea of the feature limiting frameworks, but do we actually need it? I mean I get the idea and I like it, however, if you are going to trust the code more because it uses such framework, you'll need to read the code and see that it does, so you have to read the code either way. On the other hand, if you want to include malicious code, you will just remove the framework usage. Is there a practical reason we need this? Like wanting scrot to work on a certain OS, and it not being possible because software without such framework is required, or?

[guijan]

If do we those two, I plan on forking scrot so I can remove those two, and use that version.

Is this really that bad?

[ifohancroft]

Sorry, in my opinion it really is.