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.