GNOME Bugzilla – Bug 565907
default filename for screenshot should include date and/or time
Last modified: 2011-06-13 18:54:24 UTC
There was a request for better default screenshot filename at ubuntu's brainstorm http://brainstorm.ubuntu.com/idea/16850/ . I created a rather generic implementation, patch will be attached, which uses @-escapes to denote user, hostname, display, window title, _("Screenshot") and number and then uses %-escapes that are passed to strftime. The template is stored in gconf and has default that mimics the old behavior.
Created attachment 125465 [details] [review] implement template-based file naming scheme This patch needs code review, since I am not fluent with glib string functions and use regular libc functions. I also wasn't sure about how to report failures of library calls (localtime etc.).
Created attachment 189663 [details] [review] Add a timestamp to screenshot filenames by default This is useful for correct ordering of sequences, unique filename generation regardless of target directory, and information recording.
Created attachment 189664 [details] [review] Don't use window title in default filenames Now that we have the timestamp it isn't needed to help make the filename unique. It makes the filename really really long and makes the window snaps inconsistent from the area and full screen ones. This makes it a bit harder to edit name name if necessary in the dialog.
Created attachment 189665 [details] [review] Increase the size of the filename entry To accomodate at least the timestamp.
Review of attachment 189663 [details] [review]: ::: gnome-screenshot/gnome-screenshot.c @@ +984,3 @@ + * It is used to display the time in 24-hours format (eg, like + * in France: 20:10). */ + timestamp = g_date_time_format (d, _("%Y-%m-%d %H:%M:%S")); do we really need the second as well? taking the screenshot twice will most likely take more than one second. on the other hand, the second would be perfect to avoid collisions. @@ +993,3 @@ if (job->iteration == 0) { + file_name = g_strdup_printf (_("Screenshot %s - %s.png"), timestamp, window_title); I'd go for "Screen captured - %(timestamp)s" for all cases (whole desktop, window, area) without using the window title at all. @@ +1002,3 @@ + file_name = g_strdup_printf (_("Screenshot %s - %s-%d.png"), + timestamp, + window_title, if we use seconds then the probability of a collision is nil. I'd just remove this code.
Review of attachment 189664 [details] [review]: mmh, I do wonder if we shouldn't just squash all patches :-)
Thanks for the review. I thought about using "capture" instead of screenshot actually but a few things changed my mind: * We expose "screenshot" in the UI in various places including the keyboard shortcuts panel * The GNOME docs team suggests Screenshot * I did a quick survey of some non-GNOME using friends and they seemed to use screenshot as well * Small dose of consistency with existing files (weak argument) I think having the seconds is a pretty reasonable way to avoid collisions. I know I take multiple shots per minute regularly. I'm fine with removing the per second version number if that's what you want. Though I'm not sure it hurts to have it either. Perhaps in the case where someone scripts the tool... dunno.
(In reply to comment #7) > Thanks for the review. I thought about using "capture" instead of screenshot > actually but a few things changed my mind: > > * We expose "screenshot" in the UI in various places including the keyboard > shortcuts panel > * The GNOME docs team suggests Screenshot > * I did a quick survey of some non-GNOME using friends and they seemed to use > screenshot as well > * Small dose of consistency with existing files (weak argument) I'm overly sensitive to this mostly because every time I write "screenshot" it doesn't get recognised by the spellchecker and I think I'm writing incorrectly. :-) > I think having the seconds is a pretty reasonable way to avoid collisions. yes, I realised that after I saw the second patch. > I > know I take multiple shots per minute regularly. I'm fine with removing the per > second version number if that's what you want. Though I'm not sure it hurts to > have it either. Perhaps in the case where someone scripts the tool... dunno. you cannot really script gnome-screenshot at the moment: there is no way to pass the output file, which is the only way for a script to actually know the path to the file (it's only available in the UI). if we had a "-o|--output" option then we could just check for an existing file before actually taking the screenshot, and ask for a new name or show an error dialog.
Ok after some more conversation with Cosimo we thought it still might be a good idea to keep this code since we still need to check the existence of the parent directory and stuff. I've pushed this as is and we can patch again if necessary :) Thanks!