flameshot-org / flameshot

Powerful yet simple to use screenshot software :desktop_computer: :camera_flash:
https://flameshot.org
GNU General Public License v3.0
24.7k stars 1.58k forks source link

Let launcher argument support path option #2314

Closed RedBearAK closed 2 years ago

RedBearAK commented 2 years ago

Feature Description

I have a script that passes the --path option to the CLI Flameshot arguments to automatically save screenshots into a designated location with a specific filename, built out of variables within the script. I know you can set a matching filename pattern in the Flameshot config, but the script is made to work with multiple screenshot apps, and made to avoid messing with any user settings.

With the gui, screen and full arguments, this works out great. They accept the --path option and save the file immediately in the right place with the right name.

But the launch argument says that it isn't a valid option.

I suspect that this is because the launcher is commonly used to take and save multiple screenshots. So a static filename argument would not work well. But it should be possible to bypass this issue by letting the user pass a path & filename with the same date and time variables shown in the GUI config Filename Editor. Something like this:

--path "/home/$USER/Pictures/Screenshots/Screen Shot %F at %H.%M.%S.png"

In fact it would be very nice if all the CLI arguments recognized these date variables and interpreted them appropriately when saving the screenshot file. At the moment this will just save a file literally named Screen Shot %F at %H.%M.%S.png. I make my own timestamp within the script where I call the CLI arguments, but that also means that my timestamp can be inaccurate by a significant amount if someone starts a screenshot and then has to leave their computer before completing the process, then completes the screenshot when they come back.

If Flameshot were to accept date variables in the --path option argument, then the timestamp would always be correct if the variables were evaluated at the moment the user actually saves the screenshot file.

mmahmoudian commented 2 years ago

I agree that launcher can also accept the --path. This would make our CLI more uniform.

About the date formats though, it has been already discussed about path expansions and has been decided that we always let the shell expand it and we don't do the expansion ourselves. The date format sound a lot like path expansion with much less security concerns, but also much less usefulness. From where I stand, since there is a script involved, you can save the screenshot in a temp file and then handle the naming yourself with gnu tools. Basically you either want the time of launching the launcher (which you can already give the name to the --path, or you want the time at the time of writing to disk, which if we ignore the fraction of a second that the diskIO takes, you can do the renaming yourself afterwards. So the pain to gain ratio of implementing the date formats might not worth it as it is only used for the launcher.

borgmanJeremy commented 2 years ago

I agree with @mmahmoudian on both points. Generally a good idea but no shell expansion.

RedBearAK commented 2 years ago

@mmahmoudian @borgmanJeremy

Thanks for the responses. But I'm not clear on whether you're exactly disagreeing with the usefulness or not. The date options are limited but are the only thing available in your own Filename Editor screen to begin with, and their obvious usefulness is to always have a unique filename and know when the screenshot was taken, which is why you've implemented that kind of pattern option internally, as most screenshot tools do by default.

The scenario I'm thinking of is a user triggering the start of a screenshot such as the "area selection" that the gui argument defaults to, and then getting distracted by a phone call or something. The user could be pulled away from the interrupted task for minutes, or hours, or possibly even days, and then walk back into the room and see the screen and decide to finish the screenshot process instead of escaping out of it.

I'm not against putting something more into my script, but it would seem a lot easier for the screenshot tool itself to replace the date variables when it actually completes its task.

I'm halfway decent with shell scripting, but I'm not immediately imagining exactly how I could deal with waiting for the screenshot tool to return (for potentially a long time) and only then rename and move the file. Unless a simple && would take care of the waiting for completion. But I'd probably also need to check for error codes or something? If the user cancels or kills Flameshot, there would be no file to rename.

Right now all the script has to do is launch the screenshot tool with the appropriate options and then "Get the hell out of Dodge" (i.e., "exit"). Implementing a function to wait and then check for errors and then rename/move the temporary file will make it quite a bit more complicated. The function will need to be called at every branch where I use --path. And this is what anyone who ever wants to use Flameshot from a script will have to do. If they want to make sure the timestamp is accurate.

So the pain to gain ratio of implementing the date formats might not worth it as it is only used for the launcher.

But there's no good reason to limit the date variable expansion to just the launcher argument. The example above had the user getting interrupted while using the gui argument.

Well, that's my point of view. From my perspective I would just leave the script the way it is and let the timestamp be inaccurate in the cases where there was a delay before saving the file. Effectively the main Flameshot CLI argument that would be affected by this is actually gui, since until launcher is changed to accept --path it is doing its own thing and puts an accurate timestamp on the filename, or whatever pattern the user has chosen in the GUI config, when you go to save the file(s).

But the other reason I wanted launcher to accept a --path that includes a generic filename pattern is because my script will usually be creating a filename pattern a little different from the Flameshot (or GNOME Screenshot, or Spectacle) defaults. I wanted to have more control of launcher not just saving into the same folder as the rest of the script commands, but also saving filenames with the same pattern. The only way to really do that is to let launcher accept a generic filename pattern with date variables.

An example of what my script currently does for different keyboard shortcuts:

Screen Shot 2022-01-24 at 12.47.39 fs-scrn.png
Screen Shot 2022-01-24 at 12.47.39 fs-desk.png
Screen Shot 2022-01-24 at 12.47.39 fs-area.png
Screen Shot 2022-01-24 at 12.47.39 gs-wind.png
Screen Shot 2022-01-24 at 12.47.39 fs-show.png

In this example, the suffix shows not only the tool that was used, but what type of screenshot it was; desktop, screen, area, window, or taken manually with the app GUI tool. Since Flameshot (fs) doesn't have a "window" selection mode, I fall back to using GNOME Screenshot (gs) or Spectacle (sp).

The filename at the end marked "show" (short for a show-app argument in my script) is currently impossible to generate without changing the Flameshot config, because I can't feed the filename pattern to launch the way I can with the other arguments. This also unfortunately means I can't even tell launch to save to a temporary file, as far as I know. I don't think it would work reliably to try and just redirect the output with >> into a file, right? At least not without checking for error codes in the return.

borgmanJeremy commented 2 years ago

Sorry I'm not follow the premise or goal here. Why would you use the launcher option from a script? Why not call flameshot gui directly?

If you want to save the filename with a specific pattern, set the filename pattern in the config. You can even adjust the filename pattern from the CLI with flameshot config -f image

This will timestamp it with the timestamp at the time of invocation, which makes more sense IMO since that will correlate to the time the screenshot was taken. The time at which it is saved to disk isn't really relevant.

However, if you do want the file name to use the time at which it saves to disk you can use the --raw option inside of a script.

RedBearAK commented 2 years ago

@borgmanJeremy

The purpose:

MacOS has three keyboard shortcuts that use a built-in tool for taking screenshots. It's very simple, and fast, with very little interaction. Desktop (all monitors), area selection, and window (toggles from the area selection tool with Spacebar). I use a keyboard remapper to make my keyboard in Linux work like my main computer(s), which are Macs and have been for a long time. The remapper works amazingly well to make Linux behave like macOS. But without this script I'm building, it can't easily mimic the actions of the macOS screenshot shortcuts that I've been using for many years. Not in a portable way that doesn't require setting up custom system shortcuts for each DE, and would work as I jump from distro to distro and computer to computer. You see, this remapper is really just a light "overlay" that changes very few pre-existing settings on the system where it is installed. So you can disable it and everything just goes back to normal. It's quite brilliant. (I didn't make it.)

I'm trying to make this screenshot script instantly usable by anyone who uses this keyboard remapper tool (Kinto.sh by Ben Reaves), so that when they activate the remapper they will just automatically have these "macOS" screenshot shortcuts that they may have in muscle memory, and the shortcuts will perform the actions the user expects. Since it will be distributed, I can't know what settings another user may have for Flameshot, and I don't want to mess with them. Because that would be rude. So changing the actual config of Flameshot would not really solve my problem in making the script work as expected.

The main actions of the shortcuts are no real problem, but one of the shortcuts (Cmd+Shift+5) brings up an interactive screenshot tool. The closest equivalent to this, if the Linux system has only Flameshot installed (or Flameshot is set as the user's preferred tool over Spectacle or GNOME Screenshot), is flameshot launcher. The flameshot gui argument is more like the "area selection" shortcut in macOS. That is why I am attempting to use launcher from a script. It's a CLI argument, so why not?

Unfortunately both Spectacle and GNOME Screenshot also don't seem to pay any attention to their equivalent of --path either, when you bring up their equivalent of the launcher window, so it's not like launcher ignoring --path is an unusual behavior. But if launcher could evolve to interpret the --path option and accept a generic filename pattern with date variables provided via CLI without interacting with the existing user setting, that would be a tremendously useful feature. Not just for me but for anyone who just wants to be able to get a Flameshot launcher to behave differently (different save location and/or filename construction) for any particular purpose.

Maybe somebody out there does online tutorials or something similar, and they want to set up a quick script that starts the launch window in a way that will maintain a different filename pattern and a different save location while they're working on the tutorial, but they don't want to be constantly changing their actual default settings for when they take unrelated screenshots. I don't know. I'm sure other Flameshot users would appreciate the feature I'm requesting, for reasons I can't even come up with.

I do not immediately know how to use --raw correctly, but I will look into it.

Thanks for the feedback.

This will timestamp it with the timestamp at the time of invocation, which makes more sense IMO since that will correlate to the time the screenshot was taken. The time at which it is saved to disk isn't really relevant.

Correct, the important time is when the screenshot is taken, but that will normally be quickly followed by the saving of the file, at least with most of my CLI commands. But the time of the screenshot is really what I meant. I didn't really think of that as different from when the file is saved, but of course it could be.

And that actually brings up the fact that the timestamp of my script, if I waited until a temporary file was saved, could come at a time long after the screenshot was actually taken. So really, once again, the best place for the date/time to be set is within the screenshot tool.

borgmanJeremy commented 2 years ago

Okay thanks for clarifying the issue. I can understand why you are trying to do this for your script, but it doesn't really match our workflow or intended use cases. For most of our users / use cases its perfectly fine to modify the config file to configure the program.

veracioux commented 2 years ago

@borgmanJeremy @mmahmoudian

First of all, I think we should add all or almost all options that GUI supports to the launcher.

If you agree with that, then couldn't we add another option to all screenshot subcommands called --path-format that accepts strf?

@RedBearAK I'm sorry that we can't accommodate your use case at the moment. In addition to the proposals above, I have in mind a feature where you can add a --config FILE option to flameshot with which you can specify an additional config FILE to load on top of the default user config (i.e. override the values you choose). I have yet to create a feature request for it and discuss it with the other devs.

RedBearAK commented 2 years ago

@veracioux

--config FILE

Ha! I was actually just pondering if I could duplicate the Flameshot binary somewhere (maybe even in /tmp) and then feed it an alternative config file, to keep whatever I do completely separate from normal user settings.

That kind of CLI option would be incredibly powerful. That imaginary scenario I came up with, of someone doing hundreds of screenshots with a pre-specified location and filename pattern, would be a cinch after that.

I would very much appreciate if you followed through on that, or anything similar.

--path-format

This also sounds like an excellent idea. And would allow the expansion of variables to be kept separate from the usual feeding in of a static path/filename with --path, which might actually be a good idea in and of itself. Thanks for your input on this.

borgmanJeremy commented 2 years ago

@veracioux no I don't think we should add --path-option. That can already be specified with --path.

Strf can simply be handled via shell expansion.

borgmanJeremy commented 2 years ago

@RedBearAK if you want to copy the binary you should use the appimage which is self contained with all the dependencies.

RedBearAK commented 2 years ago

@borgmanJeremy

@RedBearAK if you want to copy the binary you should use the appimage which is self contained with all the dependencies.

I already have the AppImage on my machine, but I would not want to be installing software on other people's computers that they didn't ask for. And, unless the AppImage were to be using a different config file from the regular binary, it wouldn't actually help me out. They seem to be using the same config file. I can start them independently and make a change to the config, quit and start the other version, and the change is reflected in the config.

A --config option can solve a great many things. I can use a separate config file to suppress notifications, suppress the tray icon, safely enable autoCloseIdleDaemon specifically for my own "run" of Flameshot from CLI, and so forth. Or, no, I think that last part would still kill an existing Flameshot daemon when my CLI task shuts down, until some other change is made. But a separate config file would make a huge difference in how much control the user can have over calling Flameshot from a script.