Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Add path for screenshots #139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheKingOfLight
Copy link

The path for screenshots was discussed in PR #117.

A possible solution could be to define the path for the file location as a variable in the config and then pass it to grim.
Example of how it would be saved: $XDG_PICTURES_DIR/2024-02-09_13-51-47.png instead of $XDG_PICTURES_DIR/20240209_13h51m47s_grim.png.

Advantages:

  • The directory and name of the screenshots can be easily changed.
  • Would give the possibility to further manipulate the name

e.g. focused window is saved with window name:

set $screenshot_dir $(xdg-user-dir PICTURES)/$(date +%Y-%m-%d_%H-%M-%S)
set $screenshot_focused_window $focused_window | grim -g- $screenshot_dir_$(swaymsg -t get_tree | jq -j '.. | select(.type?) | select(.focused).app_id').png

This would still not save the images in the screenshots subdirectory, because it is not clear whether the user has this directory.

Not sure if you are interested in this change.

Add a variable that contains the path and name for screenshots to be saved.
@FilippoBonazziSUSE
Copy link
Collaborator

Hi, thanks for the contributions. A couple of comments here.

I think the two issues of destination folder and screenshot name are separate.

I still believe that the best default screenshot destination folder we could achieve would be to set the XDG folder variables (they are currently unset in openSUSEway), create a $XDG_PICTURES_DIR/screenshots folder, and set GRIM_DEFAULT_DIR to this folder. This would take care of localization and would work in the general case.

The best overall option for a screenshot destination folder would be to advise the user to set GRIM_DEFAULT_DIR to their preference. The simplest way would be to document how to do this, ideally via environment.d as we already use that in openSUSEway (#82). I'm not sure this could be achieved in a turn-key fashion. We could have something like a openSUSEway first launch wizard, which could ask the user to set this variable (and possibly others we come up with) but that sounds like a quite ambitious project for a very limited return. And 99% of users would set ~/Pictures/screenshots anyway ;)

Naming the screenshot with the window name sounds neat, but it could be problematic (long titles, invalid filenames, duplicate filenames if saved without a timestamp, ...). I'm not sure I would want it to be the default.

@TheKingOfLight
Copy link
Author

I understand that you would prefer to set the directory via the environment variable. Especially in view of the fact that the directory may be customizable.
This was just a suggestion for a "simpler" implementation.

I think the two issues of destination folder and screenshot name are separate.

As far as I know, you cannot change the name of the screenshot if the environment variable GRIM_DEFAULT_DIR is used. In this case you would be bound to the format "DATE_TIME_grim.png".

@@ -50,25 +50,27 @@ bindsym --no-warn XF86AudioNext exec playerctl next
bindsym --no-warn XF86AudioPrev exec playerctl previous

# Screenshots
## Screenshot file path
set $screenshot_dir $(xdg-user-dir PICTURES)/$(date +%Y-%m-%d_%H-%M-%S).png
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdden@stdlen:~> xdg-user-dir PICTURES

The program 'xdg-user-dir' can be found in the following package:
  * xdg-user-dirs [ path: /usr/bin/xdg-user-dir, repository: download.opensuse.org-oss ]

Try installing with:
    sudo zypper install xdg-user-dirs

this needs additional dependency - please also add to the spec file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW - how to set those dirs to some defaults?

stdden@stdlen:~> xdg-user-dir PICTURES
/home/stdden

@FilippoBonazziSUSE
Copy link
Collaborator

As far as I know, you cannot change the name of the screenshot if the environment variable GRIM_DEFAULT_DIR is used. In this case you would be bound to the format "DATE_TIME_grim.png".

Huh, from the manpage it appears you're right. I haven't tried it. I see what you mean that if you want to specify a custom name, you would have to specify the output path in the filename as well, making the environment variable kind of pointless.

I am not convinced that customising the screenshot name is a good default for all users. Also, to undo that change, they would need to redefine all screenshot commands. If we just set a default GRIM_DEFAULT_DIR, they would just need to override that.

@TheKingOfLight
Copy link
Author

You're right, that sounds like a more sensible solution.
As the xdg directorys have not yet been configured, this would probably have to be solved first.

I no longer believe that the approach I presented makes sense.

@FilippoBonazziSUSE
Copy link
Collaborator

Thanks for the discussion. Perhaps we can take the opportunity to set up the XDG directories and carry the solution forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants