Some CLI option combinations load Flameshot app instead of doing "one off" invocation

[RedBearAK]

Flameshot Version

Tested with the Ubuntu DEB of the latest version as of right now: v11.0.0

Installation Type

Using the ready-made package from Github Releases

Operating System type and version

Ubuntu 21.04, 64-bit

Description

The change notes for v11 say that the CLI options have been reworked to make it easier to do "one off" invocations of Flameshot, without actually loading the app and leaving it in memory persistently. This seems to be true for most of the CLI options I've tested. But with some arguments, and especially when I use the "--clipboard" option, it is still loading Flameshot up into the top bar and shows the usual "Here I am!" notification that you get when you just start Flameshot from a GUI app launcher.

This is true for calling Flameshot in the terminal and from within a bash script.

Holdover bugs, perhaps, from the way the CLI options used to work?

Steps to reproduce

First, quit the Flameshot app from the indicator icon, if it's currently running.

1. run "flameshot gui --path "/home/$SER/Pictures/filename.png"

After running this, Flameshot GUI will let you make a rectangular selection and then immediately save the file when you hit the "Enter" key. Flameshot does not load into the Ubuntu/GNOME top bar or remain persistent in the background.

2. run "flameshot gui --clipboard" or "flameshot gui" with no further "--" options added.

After running either of these commands, Flameshot GUI will let you make a rectangular selection just like before, but after hitting "Enter" key to send the image to the clipboard (if you used "--clipboard"), Flameshot will load up into the Ubuntu/GNOME top bar and remain persistent in the background. If you used "gui" alone, and save the file with Ctrl+S, Flameshot loads up persistently.

It seems Flameshot loads persistently only after saving the clipboard data or saving the file. If you hit "Escape" to avoid saving the screenshot, a notification still appears ("Screenshot aborted") but Flameshot doesn't load persistently.

Combining "--clipboard" with "--path" will also cause Flameshot to load persistently. But with just "gui --path" it doesn't.

This doesn't happen with the "launcher" argument. The "--clipboard" option is not yet valid for the "launcher" argument, so I can't test that.

Doesn't happen with "screen", unless you add "--clipboard" ("screen --clipboard").

This doesn't happen with the "full" argument, unless you add "--clipboard" to it. ("full --clipboard")

Those are all the arguments I see available in v11.0.0.

Screenshots or screen recordings

No response

System Information

1. Ubuntu 21.04, 64-bit
2. Acer 1080p laptop screen
3. GNOME 3.38.x with Xorg

[mmahmoudian]

If you used "gui" alone, and save the file with Ctrl+S, Flameshot loads up persistently.

This👆🏼 is the only potential bug

I can see that we have forgot to mention in the release notes about the exception. As far as I understand/remember, the "one off" mode works unless we have to retain something due to the user's request. Pinning (--pin), uploading (--upload), and copying to clipboard (--clipboard) all would keep Flameshot in the memory as long as we have to keep the pin window open, while the upload process is in progress and as long as we have to keep the image in the clipboard. The moment you overwrite the clipboard the Flameshot process should be gone. The same is with flameshot gui and then you press the copy to clipboard button.

@veracioux & @borgmanJeremy please correct me if I'm wrong.

[RedBearAK]

@mmahmoudian

I see, although that is a bit unexpected. The pinning and uploading I can understand, but I guess I don't understand why putting an image in the clipboard would need Flameshot to hang around until it's gone. Unless the system clipboard doesn't handle images without assistance. I know nothing about how the clipboard works.

Just to clarify, if the user has Flameshot already running in the background, using any of these CLI options from a script shouldn't cause the pre-existing Flameshot process to shut down, or be affected in any other way, correct? In other words, if Flameshot is not running I don't want any CLI commands to start it and leave it running, and if Flameshot was already running I don't want any CLI commands to accidentally end up killing the persistent Flameshot process that was already in memory.

as long as we have to keep the image in the clipboard. The moment you overwrite the clipboard the Flameshot process should be gone.

I tested this idea that the running Flameshot would disappear after the clipboard is cleared, but that doesn't seem to happen. I pasted the image into LibreOffice Writer and then selected and copied some text into the clipboard (which should have replaced the image), then tried selecting and copying the image again in Writer, but Flameshot continues to persist in the Ubuntu/GNOME top bar. Unless I'm doing something wrong, it doesn't seem to be temporary. And then there is the "Hello, I'm here!" notification, which seems to indicate that Flameshot thinks it's starting up in a normal "Hey, I'm open for business, let's do lots of screenshots!" sort of mode.

Looking forward to clarification.

[veracioux]

@RedBearAK

Unless the system clipboard doesn't handle images without assistance.

This. But only on Xorg. Wayland doesn't have this behavior.

Just to clarify, if the user has Flameshot already running in the background, using any of these CLI options from a script shouldn't cause the pre-existing Flameshot process to shut down, or be affected in any other way, correct? In other words, if Flameshot is not running I don't want any CLI commands to start it and leave it running, and if Flameshot was already running I don't want any CLI commands to accidentally end up killing the persistent Flameshot process that was already in memory.

The daemon will not be started unless the specific operation requires the daemon to run in order to support it (pin, clipboard on X11 as already discussed). If the daemon is already running then it won't be restarted or anything. It will just receive new instructions on what it has to do.

If the daemon is already running, then it won't be killed if the config option autoCloseIdleDaemon is set to false. If true, external clipboard changes and closing the pinned widget will close the daemon. Basically we are assuming that users will want the daemon to run either all the time or never. If the former, they should set autoCloseIdleDaemon to false, if the latter they should set it to true. I think this is a fair (but not perfect) assumption.

The alternative would be to pass some additional flags when auto-launching the daemon so that it knows if it was launched to persist or just temporarily to assist with a specific command. But I am not very experienced with dbus, and don't know if the auto-launch service can be customized that way. @borgmanJeremy may know more about this. If this is possible then we could also customize it not to show the systray icon and the I'm here message if it was only launched temporarily.

I tested this idea that the running Flameshot would disappear after the clipboard is cleared, but that doesn't seem to happen.

Please make sure that autoCloseIdleDaemon is set to true. Even then, you have to copy something else to the clipboard twice in order for the daemon to quit, which is clearly a bug and we should address it.

[RedBearAK]

@veracioux

Thanks, that cleared up some things, but...

This. But only on Xorg. Wayland doesn't have this behavior.

I see. And the app I'm working with only runs on X11/Xorg. So temporary persistence is unavoidable.

Please make sure that autoCloseIdleDaemon is set to true.

Okay, this is like the fourth time I've rewritten this as I test different things and try to understand what's happening.

So you have this option, autoCloseIdleDaemon. It can be set from GUI or CLI.

If I activate this option, it will unfortunately end up killing a pre-existing running Flameshot daemon if I run a command like flameshot gui --clipboard from a script or terminal, and then empty the clipboard. I just tested this, by having the option enabled, quitting Flameshot, then launching it from a GUI app launcher to leave it in the top bar. After then running the CLI --clipboard command and pushing the image out of the clipboard, it killed the Flameshot daemon that I had launched via a GUI app launcher prior to running the script command. This is exactly what I was worried about. This sort of result will inconvenience the Flameshot user, if they happen to want Flameshot to be running, and had it running before I ran my background CLI command. (These commands will be triggered through another app by some keyboard shortcuts.)

But if I don't activate this "auto close" option, then Flameshot will remain persistent no matter what, even if it was not running as a daemon before I ran the CLI command. This may confuse the user, if they had no intention of having Flameshot always running.

Unless I'm missing something (which is a common occurrence), there does not appear to be a safe (i.e., easily portable to other systems where users have their own way of using Flameshot) method to run these CLI options without messing with the existing state of Flameshot at the moment the command is executed. The autoCloseIdleDaemon seems to sort of get only halfway to being a solution for this. This is from your "always" or "never" point of view. But I have to work from a "sometimes yes, sometimes no, and don't mess with the way it is set up" point of view. Even if this was just for my personal use, this is the way I would want it to work. Leave no footprints in the sand, as it were. But this is meant to be portable, to systems where I don't want to be messing with any kind of Flameshot user setting.

Is there not a simple way to say, "If called via CLI with options, run independently, or at least don't change existing state of daemon"?

If not, I'll just have to never use --clipboard, --pin, or --upload in my script. Because I can't be messing with other people's Flameshot config options.

What about adding options like --nokillexisting and --temporary (or --background), which if combined will still auto-close the daemon after a task if the CLI command actually had to start the daemon itself, but negate that end result if the daemon was already in memory when the command was executed? The --temporary (or --background) option could also be used to suppress irrelevant behaviors like inserting the indicator icon in the indicator tray, and showing the "Here I am!" notification. Making temporary background usage of Flameshot much more transparent.

Here is some good news, at least. I don't think I can replicate Flameshot becoming persistent with just the gui argument alone. I may have confused myself during testing by thinking that a command from my script which included gui --clipboard was just using gui. So at least when I leave --clipboard out, this doesn't seem to be a problem with any of the main CLI arguments. But I would sure appreciate having cleaner/safer access to at least the --clipboard option, but possibly also --pin and --upload. Without stepping on the user's toes (their config settings, or the existing state of the daemon).

[veracioux]

@RedBearAK I agree completely with you, and have always agreed. But I didn't know of a way to make it work that way without introducing a new CLI option. But now I think I know a solution that doesn't require a CLI option.

@borgmanJeremy Can we change the Exec line in data/dbus/org.flameshot.Flameshot.service.in to something like:

Exec=__FLAMESHOT_DAEMON_TEMPORARY__=true ${CMAKE_INSTALL_FULL_BINDIR}/flameshot

I'm not sure if the syntax is proper, i.e. that environment variables can be set this way in the service script.

Then we can check in FlameshotDaemon::start if this env variable is set, and force m_persist to false if it is. Now the daemon is non-persistent if auto-launched by DBus, otherwise it reads autoCloseIdleDaemon from the config. Additionally we can disable the systray icon and the hello message in this case.

If the user launches flameshot manually while the temporary daemon is active, we can send a DBus call to the temporary daemon, instructing it to make itself persistent.

It could still be the case that the user wants the daemon to run even if it was auto-launched, but I think sensible users will either run the daemon manually by flameshot or by adding it as a startup program. Therefore I think we can consider this an edge case.

@borgmanJeremy @mmahomoudian Is this a viable approach?

[RedBearAK]

@veracioux @borgmanJeremy @mmahmoudian

There are examples here of using Environment= lines (one for each variable?) under the [Service] header to insert environment variables into a systemd unit service file, if that 's what you're talking about. One method creates one of those something.d directories with a conf file where you put the environment lines, the other way is just to drop it right into the same service file.

https://www.thegeekdiary.com/how-to-set-environment-variables-for-a-systemd-service-in-centos-rhel-7/

https://www.flatcar.org/docs/latest/setup/systemd/environment-variables/

This may or may not be a "newer" way, as opposed to using EnvironmentFile=, or maybe EnvironmentFile just came first and both are equally valid. I don't know.

https://serverfault.com/questions/413397/how-to-set-environment-variable-in-systemd-service

[borgmanJeremy]

I dont think either of these will be cross platform.I guess I don't want to sink a bunch of complexity in this just so the clipboard doesn't run in the daemon.

[RedBearAK]

@borgmanJeremy

I dont think either of these will be cross platform.I guess I don't want to sink a bunch of complexity in this just so the clipboard doesn't run in the daemon.

Unless I'm completely misunderstood the situation, I don't think that's what this is about. This is about avoiding letting the clipboard/pin/upload CLI options cause a pre-existing daemon to be killed inappropriately (if autoCloseIdleDaemon happens to be enabled but the CLI argument didn't start the daemon), and not letting the CLI arguments make the daemon persistent if it wasn't already persistent and in memory when the CLI command was run.

The temporary loading of the daemon while the clipboard/pin/upload task is happening would seem to be unavoidable. Which would be fine, except for the unwanted effects the temporary tasks have on an existing persistent daemon (if autoCloseIdleDaemon is enabled), and on leaving a persistent daemon around in an unwanted way (if autoCloseIdleDaemon is disabled). Currently, there is no happy middle ground when using the CLI options in a portable script where you can't know how the user actually likes to use Flameshot, and whether autoCloseIdleDaemon is enabled or disabled.

The suggestion by @veracioux would seem to be a simple approach to at least solving this on Linux. I don't even know if this is actually a problem on the other platforms. I'm only testing on Linux, with Xorg.

[borgmanJeremy]

Ah OK now I understand. Yes I agree the pre-existing daemon should not be killed by an innovation of the CLI. However what ever the solution is it must be cross platform.

[RedBearAK]

@borgmanJeremy

Ah OK now I understand. Yes I agree the pre-existing daemon should not be killed by an innovation of the CLI. However what ever the solution is it must be cross platform.

Well, yes. If the logic is broken on all platforms and leaving a persistent daemon or killing a persistent daemon at times when the user doesn't want that, I'd very strongly advocate for fixing that.

I guess I was assuming this was probably only a real problem in Linux, due to the way the daemon is loaded and then shut down when using these options. But I don't really have strong evidence for that. Haven't tested on the other platforms.

[borgmanJeremy]

I took a closer look at this today and while the behavior is not correct for your scripting use case, it is working as intended. The option explicitly says "close daemon when not needed". If the users of your script do not want the tray icon to close, they should not use this option.

That said, I think a good middle ground would be to add a --single-shot CLI flag. @veracioux I think we could set it up so it doesn't use SingleApplication and instead spawns its own process. This would work on all platforms and be simple to add.

[veracioux]

@borgmanJeremy The issue that is being discussed here is only a Linux issue. On Windows, the daemon is required at all times anyway. On MacOS, actions that require flameshot to persist simply prevent the current flameshot process from exiting until persistence is no longer necessary (e.g. user closes the pin widget). In other words, they don't launch the daemon. The problem with this is that the process that initiated the action (e.g. flameshot gui) doesn't exit immediately, which makes it inconsistent between Linux and MacOS. Looking back at this I realize that this wasn't the best design choice on my part.

I suggest another (and hopefully final for all time) modification:

Actions that must be backed by a running flameshot instance shall cause the current instance to fork. For example:

• User runs flameshot gui --pin (PID X), selects a region and presses Enter/"Accept capture"
• The flameshot process X forks, creating another flameshot process with PID Y, which is responsible for hosting the pin until the user closes it

Pros:

• Cross platform
• Probably enables us to get rid of dbus as a flameshot internal dependency (will still be necessary for notifications etc.)

Cons:

• Each final action will cause another instance of flameshot to be launched (forked). We can minimize the performance/memory impact of this by deallocating unneeded stuff from the forked instance.