Feature: Save as dialog

[Maaxxs]

As mentioned in issue #125 it would be nice to save images to a different location than the default.
I thought so, too and came up with the following solution.
I tried to keep everything clean and simple.

• Add a button next to the Save button with the "official" save_as image.
• this creates a FileChooserDialog.
• the suggested location and filename in the dialog uses the folder and file format specified in the configuration. In order to make this possible I put the part which generates the formatted filename in its own function to reuse it.

Todo:

• I'd like to add the shortcut ctrl+shift+s for the save_as dialog. I think that'd be useful, too.

[jtheoof]

Thanks for your contribution @Maaxxs. I'm not against it but there are few things to change.

1. Can you squash all your commits into 1?
2. Make sure clang-format is run on all the files you're changing.
3. Please have a look at my comments and suggestions.

[Maaxxs]

thanks for your suggestions!
The _ is now replaced with a - in the .glade file as you suggested.

Other points:

1. commits are squashed into one.
2. I ran clang-format. Totally forgot that. Looks much better now 👍

todo:

• I set the title of the chooser dialog to null since I think it's not displayed actually. Do you agree?
• translation for cancel and save text? I think this requires some more setup for the C files?

[jtheoof]

Is there a reason why you're putting the "_Cancel" and "_Save" between parenthesis?

Regarding translating those, I'm not sure. The gtk doc seems to indicate what you are doing is valid. But I don't know if it means it's properly translated and using the base gtk po files. I'd need to research.

[jtheoof]

I've switched to French, and the Close / Save do not seem to be translated. You'll have to use gettext I guess? I don't know if the ninja target will be smart enough to catch those. (I don't think there any translation done in the .c files. It's only in the ui files for now.)

Good luck. :)

[Maaxxs]

I had some time again, so I continued here.
so _("_Cancel") and similar need to be translated as usual, afaiu. (example in use in thunar with the translation)

I added some code for the initialization of textdomain and similar. I just realized that I can use the variable GETTEXT_PACKAGE from src/po/meson.build. I take another look at this tomorrow.
But I'm working on this! :)

The initial step to include the translation for the dialog buttons is done.

[Maaxxs]

  // set locales according to environment variables
  setlocale(LC_ALL, "");

The default of LC_ALL is C. How the new value is set is well described in the manpage. quote:

If locale is an empty string, "", each part of the locale that should be modified is set according to the environment variables. The details are implementation-dependent. For glibc, first (regardless of catego- ry), the environment variable LC_ALL is inspected, next the environment variable with the same name as the category (see the table above), and finally the environment variable LANG. The first existing environment variable is used. If its value is not a valid locale specification, the locale is unchanged, and setlocale() returns NULL.

The locale "C" or "POSIX" is a portable locale; it exists on all con- forming systems.

I think it would be nice to do this, although it's not necessary as swappy is not using time, number units or similar where the formatting depends on the locale. AFAIU, this would enable an user of swappy to just set LC_ALL=de_DE.UTF-8 and all locales would be set to this value such as time, units, language ...
Example:

$ LC_ALL=de_DE swappy -f ~/Pictures/swappytest.png
$ LANG=de_DE swappy -f ~/Pictures/swappytest.png
$ LC_ALL=en_US LANG=de_DE swappy -f ~/Pictures/swappytest.png
$ LC_ALL=en_US LANG=de_DE LANGUAGE=fr_FR swappy -f ~/Pictures/swappytest.png

The first two commands set the language to German. The third one sets it to English because (according to the manpage)

The first existing environment variable is used.

However, if LANGUAGE is specified, it's always used. Hence, the language with the last command is French.

I'm happy to remove this if you don't agree.

---

  // set base directory for translated messages
  bindtextdomain(GETTEXT_PACKAGE, LOCALEDIR);

This is the most important call that caused me some headache, too. The default prefix on my archlinux system is /usr/local/. When I built and installed swappy, it would look for translations in /usr/share/locale and not in /usr/local/share/locale. With this call the locale directory is set to the correct one based on the prefix, e.g. /usr/ or /tmp/testroot.

---

  // explicitly set encoding of message translations to UTF-8
  bind_textdomain_codeset(GETTEXT_PACKAGE, "UTF-8");

this forces the codeset to UTF-8. I think this is ok as the translation files are all UTF-8 as well. For instance, if we don't do this, then I get the following warnings if only de_DE instead of de_DE.UTF-8 is specified.

$ LC_ALL=de_DE swappy -f ~/Pictures/swappytest.png

(swappy:179132): Pango-WARNING **: 14:07:42.612: Invalid UTF-8 string passed to pango_layout_set_text()
(swappy:179132): Pango-WARNING **: 14:07:42.624: Invalid UTF-8 string passed to pango_layout_set_text()

---

btw, should I continue to force push changes in that one commit or squash the commits in the end? (I always feel kinda bad when doing a force push :) ).

[jtheoof]

Thank you @Maaxxs. I'll have a look when I get some time.

You can definitely keep "force pushing", it's your branch on your own fork you can do whatever you want with it.

[Maaxxs]

Thank you @Maaxxs. I'll have a look when I get some time.

Sure. Thanks!

You can definitely keep "force pushing", it's your branch on your own fork you can do whatever you want with it.

Ok. My thinking was that it may be easier for you to review changes. And I think GitHub get confused sometimes when you do a force push and there are open reviews.
So if you don't mind, I'll do normal commits and squash them in the end so that only one commit gets merged.

[Maaxxs]

hi @jtheoof have you had time for another look at this?

[Maaxxs]

nice to see some activity here. Not sure what I can do to push this further.

I'm running with these changes since I proposed them and so far (~2 years :) ) it works well.

[leon-erd]

Would be really nice to see indeed!

[rubiin]

I am not sure whats causing the delay either, lets wait for the review

[gebeer]

I'd really love to have this feature, too. Would make swappy an even greater replacement for flameshot.

[rubiin]

@Maaxxs can you ask for review once more.
cc @jtheoof

[enzi]

I just looked up if Snappy has this feature.
Doesn't look like @jtheoof will merge anytime soon, so I compiled your fork. Thanks @Maaxxs !