Merge lp:~djaler1/screenshot-tool/non-interactive-mode into lp:~elementary-apps/screenshot-tool/trunk

Proposed by Kirill Romanov
Status: Merged
Approved by: Fabio Zaramella
Approved revision: 303
Merged at revision: 307
Proposed branch: lp:~djaler1/screenshot-tool/non-interactive-mode
Merge into: lp:~elementary-apps/screenshot-tool/trunk
Diff against target: 254 lines (+95/-57)
2 files modified
src/Screenshot.vala (+4/-2)
src/ScreenshotWindow.vala (+91/-55)
To merge this branch: bzr merge lp:~djaler1/screenshot-tool/non-interactive-mode
Reviewer Review Type Date Requested Status
Fabio Zaramella (community) Approve
Review via email: mp+317674@code.launchpad.net

Commit message

Implement non-interactive mode (fix bug 1665708)

Description of the change

Implement non-interactive mode (fix bug 1665708)

To post a comment you must log in.
Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

I'm experiencing two problems:
First, sometimes screenshots are saved, sometimes not, but i can't tell when file is correctly saved.
Second, ok it doesn't build up the gui but... app icon still popup in plank and then immediately close, this is really annoying in my opinion and with the problem above you still can't understand when screenshot is correctly saved.

review: Needs Fixing
Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

Furthermore, when they are saved is in the home folder not in Pictures even though this is the selected folder.

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

Oh, and choosing "current window" option sometimes results in icon being kept in plank until i click on it and then close the main program.

Apart from the fact that i can't tell whether they are saved or not and the problem with the folder, when it works, it works good.

Revision history for this message
Kirill Romanov (djaler1) wrote :

@fabiozaramella I made some changes, can you test again?

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

@djaler1 I tested it again and i can confirm that icon doesn't blink anymore, but it persists in plank (and i didn't choose to keep it in the dock). Mh, i think icon shouldn't be displayed at all, you can see how gnome-screenshot works for example.
Furthermore, screenshots aren't saved in the correct folder.

Revision history for this message
Kirill Romanov (djaler1) wrote :

It's strange, cause I didn't see icon in plank now.
And about folder - it save into folder that set in net.launchpad.screenshot.folder-dir. You can change it by gsettings, or, for example, by manual save screenshot to correct folder from GUI version of screenshot-tool

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

@djaler1 What i mean is that icon persist, for a while, and then disappears. As if main process is not terminated. Can you reproduce it? If not maybe is better to ask someone else to test it.

About folder, you're right, i looked at the string with dconf-editor and it's empty. That's the reason why they are saved in Pictures from the gui version and in the home folder otherwise.
For some reason the gui version doesn't save the string to the schema (i also tried with trunk) so this is a separate problem, maybe it also need to be set to Picture by default, but again this is a separate bug.

Revision history for this message
Kirill Romanov (djaler1) wrote :

https://youtu.be/31muJSAlxqY there are screencast. I doesn't see icon in plank

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

Ok, i figured it out:
When screenshots are taken from terminal, as in you screencast, icon doesn't show but screenshot arent' save (not even in the home folder).
When taken from context menu in slingshot the icon pops up and persists for a while and screenshots are saved in the home folder.

Revision history for this message
Kirill Romanov (djaler1) wrote :

I think this is Slingshot or Plank problem. They waiting for application window opening, but it doesn't show.

Revision history for this message
Kirill Romanov (djaler1) wrote :

And there no differnce in taken from context menu or from terminal to saving file. Context menu just execute command.

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

Yes, maybe is plank fault, I just thought it would be nice to avoid this, just like gnome does. :) It wasn't a problem before since save dialog was shown.
And, yes i know slingshot just execute the command, that's the reason why it's strange. If i actually tipe the command in terminal the result is that of your screencast, but launching it from slingshot produces a different result.

Revision history for this message
Kirill Romanov (djaler1) wrote :

I think it impossible with currect codebase. May be it's something in Granite.Application. I tried, but even if I don't create ScreenshotWindow icon pops up.

Revision history for this message
Kirill Romanov (djaler1) wrote :

Just for note - with old version of Slingshot (like from September) icon doesn't pops up.

Revision history for this message
Kirill Romanov (djaler1) wrote :

@fabiozaramella so what?

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote :

Ok after some tests i found out that this is a problem in slingshot. Icon doesn't pop up when the program is launched from terminal and the same happens when writing the command in slingshot's search field, it only occurs when choosing the option from context menu.

Sorry for the late reply, approved. :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/Screenshot.vala' (properties changed: -x to +x)
--- src/Screenshot.vala 2017-02-16 22:02:14 +0000
+++ src/Screenshot.vala 2017-02-18 22:07:43 +0000
@@ -32,17 +32,19 @@
32 private bool win = false;32 private bool win = false;
33 private bool area = false;33 private bool area = false;
34 private bool redact = false;34 private bool redact = false;
35 private bool clipboard = false;
3536
36 construct {37 construct {
37 flags |= ApplicationFlags.HANDLES_COMMAND_LINE;38 flags |= ApplicationFlags.HANDLES_COMMAND_LINE;
3839
39 options = new OptionEntry[6];40 options = new OptionEntry[7];
40 options[0] = { "window", 'w', 0, OptionArg.NONE, ref win, _("Capture active window"), null };41 options[0] = { "window", 'w', 0, OptionArg.NONE, ref win, _("Capture active window"), null };
41 options[1] = { "area", 'r', 0, OptionArg.NONE, ref area, _("Capture area"), null };42 options[1] = { "area", 'r', 0, OptionArg.NONE, ref area, _("Capture area"), null };
42 options[2] = { "screen", 's', 0, OptionArg.NONE, ref screen, _("Capture the whole screen"), null };43 options[2] = { "screen", 's', 0, OptionArg.NONE, ref screen, _("Capture the whole screen"), null };
43 options[3] = { "delay", 'd', 0, OptionArg.INT, ref delay, _("Take screenshot after specified delay"), _("Seconds")};44 options[3] = { "delay", 'd', 0, OptionArg.INT, ref delay, _("Take screenshot after specified delay"), _("Seconds")};
44 options[4] = { "grab-pointer", 'p', 0, OptionArg.NONE, ref grab_pointer, _("Include the pointer with the screenshot"), null };45 options[4] = { "grab-pointer", 'p', 0, OptionArg.NONE, ref grab_pointer, _("Include the pointer with the screenshot"), null };
45 options[5] = { "redact", 'e', 0, OptionArg.NONE, ref redact, _("Redact system text"), null };46 options[5] = { "redact", 'e', 0, OptionArg.NONE, ref redact, _("Redact system text"), null };
47 options[6] = { "clipboard", 'c', 0, OptionArg.NONE, ref clipboard, _("Save screenshot to clipboard"), null };
4648
47 add_main_option_entries (options);49 add_main_option_entries (options);
4850
@@ -151,7 +153,7 @@
151 if (action == 0) {153 if (action == 0) {
152 normal_startup ();154 normal_startup ();
153 } else {155 } else {
154 window = new ScreenshotWindow.from_cmd (action, delay, grab_pointer, redact);156 window = new ScreenshotWindow.from_cmd (action, delay, grab_pointer, redact, clipboard);
155 window.set_application (this);157 window.set_application (this);
156 window.take_clicked ();158 window.take_clicked ();
157 }159 }
158160
=== modified file 'src/ScreenshotWindow.vala' (properties changed: -x to +x)
--- src/ScreenshotWindow.vala 2017-02-17 19:10:33 +0000
+++ src/ScreenshotWindow.vala 2017-02-18 22:07:43 +0000
@@ -39,6 +39,7 @@
39 private bool close_on_save;39 private bool close_on_save;
40 private bool redact;40 private bool redact;
41 private int delay;41 private int delay;
42 private bool to_clipboard;
4243
43 public ScreenshotWindow () {44 public ScreenshotWindow () {
44 Object (border_width: 6,45 Object (border_width: 6,
@@ -50,9 +51,14 @@
50 close_on_save = settings.get_boolean ("close-on-save");51 close_on_save = settings.get_boolean ("close-on-save");
51 redact = settings.get_boolean ("redact");52 redact = settings.get_boolean ("redact");
52 delay = settings.get_int ("delay");53 delay = settings.get_int ("delay");
54 to_clipboard = false;
53 }55 }
5456
55 construct {57 construct {
58 if (from_command) {
59 return;
60 }
61
56 set_keep_above (true);62 set_keep_above (true);
57 stick ();63 stick ();
5864
@@ -164,7 +170,7 @@
164 cancel_btn.clicked.connect (cancel_clicked);170 cancel_btn.clicked.connect (cancel_clicked);
165 }171 }
166172
167 public ScreenshotWindow.from_cmd (int? action, int? delay, bool? grab_pointer, bool? redact) {173 public ScreenshotWindow.from_cmd (int? action, int? delay, bool? grab_pointer, bool? redact, bool? clipboard) {
168 if (delay != null) {174 if (delay != null) {
169 this.delay = delay;175 this.delay = delay;
170 }176 }
@@ -191,6 +197,10 @@
191 }197 }
192 }198 }
193199
200 if (clipboard != null) {
201 to_clipboard = clipboard;
202 }
203
194 close_on_save = true;204 close_on_save = true;
195 from_command = true;205 from_command = true;
196206
@@ -297,60 +307,75 @@
297307
298 play_shutter_sound ("screen-capture", _("Screenshot taken"));308 play_shutter_sound ("screen-capture", _("Screenshot taken"));
299309
300 var save_dialog = new Screenshot.Widgets.SaveDialog (screenshot, settings, this);310 if (from_command == false) {
301 save_dialog.save_response.connect ((response, folder_dir, output_name, format) => {311 var save_dialog = new Screenshot.Widgets.SaveDialog (screenshot, settings, this);
302 save_dialog.destroy ();312 save_dialog.save_response.connect ((response, folder_dir, output_name, format) => {
303313 save_dialog.destroy ();
304 if (response) {314
305 string[] formats = {".png", ".jpg", ".jpeg",".bmp", ".tiff"};315 if (response) {
306 string output = output_name;316 string[] formats = {".png", ".jpg", ".jpeg",".bmp", ".tiff"};
307317 string output = output_name;
308 foreach (string type in formats) {318
309 output = output.replace (type, "");319 foreach (string type in formats) {
310 }320 output = output.replace (type, "");
311321 }
312 string file_name = "";322
313 int attempt = 0;323 try {
314324 save_file (output, format, folder_dir, screenshot);
315 // Check if file exists325
316 do {326 if (close_on_save) {
317 if (attempt == 0) {327 this.destroy ();
318 file_name = Path.build_filename (folder_dir, "%s.%s".printf (output, format));328 }
319 } else {329 } catch (GLib.Error e) {
320 file_name = Path.build_filename (folder_dir, "%s (%d).%s".printf (output, attempt, format));330 show_error_dialog ();
321 }331 debug (e.message);
322332 }
323 attempt++;333 }
324 } while (File.new_for_path (file_name).query_exists ());334 });
325335
326 try {336 save_dialog.close.connect (() => {
327 screenshot.save (file_name, format);337 if (close_on_save) {
328338 this.destroy ();
329 if (close_on_save) {339 }
330 this.destroy ();340 });
331 }341
332 } catch (GLib.Error e) {342 save_dialog.show_all ();
333 show_error_dialog ();343 } else {
334 debug (e.message);344 if (to_clipboard) {
335 }345 Gtk.Clipboard.get_default (this.get_display ()).set_image (screenshot);
336 }346 } else {
337347 var date_time = new GLib.DateTime.now_local ().format ("%Y-%m-%d %H.%M.%S");
338 if (from_command && close_on_save) {348
339 this.destroy ();349 /// TRANSLATORS: %s represents a timestamp here
340 }350 string file_name = _("Screenshot from %s").printf (date_time);
341 });351 string folder_dir = settings.get_string ("folder-dir");
342352 string format = settings.get_string ("format");
343 save_dialog.close.connect (() => {353
344 if (close_on_save) {354 save_file (file_name, format, folder_dir, screenshot);
345 this.destroy ();355 }
346 }356 this.destroy ();
347 });357 }
348
349 save_dialog.show_all ();
350358
351 return false;359 return false;
352 }360 }
353361
362 private void save_file (string file_name, string format, string folder_dir, Gdk.Pixbuf screenshot) {
363 string full_file_name = "";
364 int attempt = 0;
365
366 do {
367 if (attempt == 0) {
368 full_file_name = Path.build_filename (folder_dir, "%s.%s".printf (file_name, format));
369 } else {
370 full_file_name = Path.build_filename (folder_dir, "%s (%d).%s".printf (file_name, attempt, format));
371 }
372
373 attempt++;
374 } while (File.new_for_path (full_file_name).query_exists ());
375
376 screenshot.save (full_file_name, format);
377 }
378
354 public void take_clicked () {379 public void take_clicked () {
355 this.set_opacity (0);380 this.set_opacity (0);
356381
@@ -371,7 +396,9 @@
371 this.hide ();396 this.hide ();
372397
373 Timeout.add_seconds (delay - (redact ? 1 : 0), () => {398 Timeout.add_seconds (delay - (redact ? 1 : 0), () => {
374 this.present ();399 if (from_command == false) {
400 this.present ();
401 }
375 return grab_save (null, redact);402 return grab_save (null, redact);
376 });403 });
377 }404 }
@@ -395,7 +422,9 @@
395 item.set_events (item.get_events () | Gdk.EventMask.STRUCTURE_MASK);422 item.set_events (item.get_events () | Gdk.EventMask.STRUCTURE_MASK);
396 }423 }
397424
398 this.present ();425 if (from_command == false) {
426 this.present ();
427 }
399428
400 if (win != null) {429 if (win != null) {
401 grab_save (win, redact);430 grab_save (win, redact);
@@ -428,16 +457,23 @@
428 if (close_on_save) {457 if (close_on_save) {
429 this.destroy ();458 this.destroy ();
430 } else {459 } else {
431 this.present ();460 if (from_command == false) {
461 this.present ();
462 }
432 }463 }
433 });464 });
434465
435 var win = selection_area.get_window ();466 var win = selection_area.get_window ();
436467
437 selection_area.captured.connect (() => {468 selection_area.captured.connect (() => {
469 if (delay == 0) {
470 selection_area.set_opacity (0);
471 }
438 selection_area.close ();472 selection_area.close ();
439 Timeout.add_seconds (delay - (redact ? 1 : 0), () => {473 Timeout.add_seconds (delay - (redact ? 1 : 0), () => {
440 this.present ();474 if (from_command == false) {
475 this.present ();
476 }
441 return grab_save (win, redact);477 return grab_save (win, redact);
442 });478 });
443 });479 });

Subscribers

People subscribed via source and target branches