GNOME Bugzilla – Bug 762306
Change default name of the file when printing map with route.
Last modified: 2018-03-26 13:08:08 UTC
When printing map with route instructions, change the default name of the file to be generated. Name : From Place to Place
Yes, we need to find a nice name for this. Do we do anything special when via-points are involved? We should not try to do to much here I think. Andreas: Any idea? For reference please look at how the "detail string", the second row of the list in search popover looks when completing to recent routes. Both with and without via points.
Following the naming convention of Shell's screenshot tool, it would be "Map of route from Place to Place.pdf" "Sunne kommun → Uppsala.pdf" could also work, as long as we skip the arrow, as that feels risky with regards to compatibility with other OSes. So that would become "Sunne kommun to Uppsala.pdf"
How should we deal with places that also include the name of the country? A few examples of places the route planner suggested for me: Houston, Harris County, Texas, United States of America Berlin, Berlin, Germany Should we just keep the first word of the name, as it is usually good enough? i.e Houston and Berlin
(In reply to Razvan Brinzea from comment #3) > How should we deal with places that also include the name of the country? > A few examples of places the route planner suggested for me: > > Houston, Harris County, Texas, United States of America > Berlin, Berlin, Germany > > Should we just keep the first word of the name, as it is usually good > enough? i.e Houston and Berlin Yes, in my opinion that is a good option to avoid longer names.And i was thinking, if we search route through right click menu, then instead of including coordinates in file name, we can keep it simple, may be just "RouteInstructions". This is because coordinates will not make very much sense to user and make the file name bit difficult to read/refer to.
I've looked into this but I can't seem to find a solution. It seems that this 'output.pdf' default is set somewhere deeper in the printing API. Maybe something could be done by using PrintOperation's set_print_settings() ( https://developer.gnome.org/gtk3/stable/gtk3-High-level-Printing-API.html#gtk-print-operation-set-print-settings ), but for this you need a previous set of print settings, and you can only get those after run() has been executed on the print operation, so after the user has selected a printer (or printed to file) and pressed the Print button. Seeing these things, I don't think it's of much use to us. Any other ideas?
Look here: https://developer.gnome.org/gtk3/stable/GtkPrintSettings.html#gtk-print-settings-set And here: https://developer.gnome.org/gtk3/stable/GtkPrintSettings.html#GTK-PRINT-SETTINGS-OUTPUT-BASENAME:CAPS And here: https://developer.gnome.org/gtk3/stable/gtk3-High-level-Printing-API.html#gtk-print-operation-get-print-settings
I've already looked into those things, but I might be missing something. From the get_print-settings() page: Note that the return value is NULL until either gtk_print_operation_set_print_settings() or gtk_print_operation_run() have been called. So we can only get a GtkPrintSettings after the print dialog has been run (so after a complete printing operation). Ideally we'd want to have that before the user ever prints, so maybe we could somehow create a set of default print settings through code when the application starts. Then, we could modify those with the corresponding basename whenever a new PrintOperation is created. Does that sound like a reasonable way to do it?
(In reply to Razvan Brinzea from comment #7) > I've already looked into those things, but I might be missing something. > > From the get_print-settings() page: > Note that the return value is NULL until either > gtk_print_operation_set_print_settings() or gtk_print_operation_run() have > been called. > > So we can only get a GtkPrintSettings after the print dialog has been run > (so after a complete printing operation). Ideally we'd want to have that > before the user ever prints, so maybe we could somehow create a set of > default print settings through code when the application starts. Then, we > could modify those with the corresponding basename whenever a new > PrintOperation is created. Does that sound like a reasonable way to do it? Do it in the _beginPrint method. That is after run and before rendering. I added: print(operation.get_print_settings()); There, and got non-null response: [object instance proxy GIName:Gtk.PrintSettings jsobj@0x7f26a21b8c10 native@0x28b3440] begin-print: Emitted after the user has finished changing print settings in the dialog, before the actual rendering starts. A typical use for ::begin-print is to use the parameters from the GtkPrintContext and paginate the document accordingly, and then set the number of pages with gtk_print_operation_set_n_pages().
Created attachment 322407 [details] File name that should be changed? I am a little bit confused, I might have misunderstood the issue maybe. What I understood is that we want to change the default name that appears when the Print to file option is selected in the print dialog (see attachment). The problem is that after run() finishes its execution, the print window is gone, so it is a bit too late to modify something. If I understood correctly what needs to be done, then I also found a solution (I should have been a bit more patient in the first place, I guess). It seems that the default print settings are equivalent to a GtkPrintSettings object with no properties set. So if I create a new GtkPrintSettings object, and then set just it's Gtk.PRINT_SETTINGS_OUTPUT_BASENAME property, it should do the job correctly, as all of the other values would be automated to default ones. I will play around with this approach, and if it seems that everything is fine, I will try to make a patch as soon as possible.
Yeah you are right, that is the right approach! No worries, check in with #gnome-design for suggestion of the file name. I do not know if it needs to be translatable. In that case since we are in string announcement period (beta of gnome is out) we need to annouce to translation list. Talk to designers!
Created attachment 322412 [details] [review] Patch for improved print file name I didn't read that there was someone else working on this but I am just uploading this patch anyway. I solved the problem with getting the settings by making the operation asynchronous.
(In reply to Razvan Brinzea from comment #3) > How should we deal with places that also include the name of the country? > A few examples of places the route planner suggested for me: > > Houston, Harris County, Texas, United States of America > Berlin, Berlin, Germany > > Should we just keep the first word of the name, as it is usually good > enough? i.e Houston and Berlin I think we only need to be as specific as needed. Taking New York, as an example, the NY in the state of New York, US. If within the same city: Main Street to Tremor Street.pdf If within the same country: Main Street, New York to Texas, Dallas.pdf If in different countries: Main Street, New York, USA to Dufferin Street, Toronto, Canada.pdf
Some more feedback needed. Some places are neither streets, towns nor countries. For example, let's take the Eiffel Tower. How should we handle this? Should we omit its name, and still go for "street, town, country" (omitting some fields if applicable, as Andreas suggested above), or also use its name. If we use its name, what would be the best approach? I'm thinking maybe we could stick to the same approach Andreas mentioned above, and replace the street with the name of the place (for such distinctive places), so we would obtain names of similar lengths and we would also give something that is closer to the user's input. Both in the same city: Avenue Franco-Russe to Eiffel Tower Both in the same country: Rue des Cordiers, Orleans to Eiffel Tower, Paris In different countries: Bulevardul Unirii, Bucharest, Romania to Eiffel Tower, Paris, France Does this sound reasonable or should I tinker with the idea a bit more?
Review of attachment 322412 [details] [review]: Thanks Elias for the patch. ::: src/printOperation.js @@ +54,3 @@ this.onAbortDialogResponse.bind(this)); + this._operation.set_allow_async(true); I think better option would be to avoid this and go with what Razvan suggested. Making an object of print setting and then setting output-basename @@ +131,3 @@ + } + if (endName.length > 20){ + endName = endName.substr(0, 17) + '\u2026'; Please check what Andreas has commented on it.We should not take substrings. Rather checking the street, city, and country will be helpful.
(In reply to Jonas Danielsson from comment #8) > Do it in the _beginPrint method. That is after run and before rendering. If setting the file name happens in the "_beginPrint" function, the user first sees the file name as "Documents/Output.pdf" and when he hits the run button, the file is actually stored in "Documents/Maps of Route from x to y.pdf". From the user perspective, I think this is rather confusing.
(In reply to Razvan Brinzea from comment #13) > Some more feedback needed. > > I'm thinking maybe we could stick to the same approach Andreas mentioned > above, and replace the street with the name of the place (for such > distinctive places), so we would obtain names of similar lengths and we > would also give something that is closer to the user's input. > > Both in the same city: > Avenue Franco-Russe to Eiffel Tower > > Both in the same country: > Rue des Cordiers, Orleans to Eiffel Tower, Paris > > In different countries: > Bulevardul Unirii, Bucharest, Romania to Eiffel Tower, Paris, France > > Does this sound reasonable or should I tinker with the idea a bit more? I like this approach.
Created attachment 323483 [details] [review] printOperation: Change default name of the output file This is probably going to need some more work, but here's what I have so far. The new _createFilename function takes an array of points that describes a route, and returns a suitable file name based on the start and end points. The three main components of the file name are street name / landmark name (only one of these two should be enough), town and country. The street name / landmark name will always appear (if it exists), and the town and country names will only appear if they are different. I've tried this out for a lot of different test cases and the results look good so far, but I might have still missed some cases. One thing that doesn't show up right now is the 'state' (applicable in case of US states and quite a few more), but the names started to get quite long so I thought I'd try it like this and then improve upon the feedback I receive.
(In reply to Razvan Brinzea from comment #18) > Created attachment 323483 [details] [review] [review] > printOperation: Change default name of the output file > > This is probably going to need some more work, but here's what I have so far. > > The new _createFilename function takes an array of points that describes a > route, and returns a suitable file name based on the start and end points. > > The three main components of the file name are street name / landmark name > (only one of these two should be enough), town and country. The street name > / landmark name will always appear (if it exists), and the town and country > names will only appear if they are different. > > I've tried this out for a lot of different test cases and the results look > good so far, but I might have still missed some cases. > > One thing that doesn't show up right now is the 'state' (applicable in case > of US states and quite a few more), but the names started to get quite long > so I thought I'd try it like this and then improve upon the feedback I > receive. Thanks! I will not review this on code level yet. My fear is that this is too complex for a very small thing. We do not need to get this perfect. We might not event want to because that would create something that is too complex in our code base. Maybe just naming it like "Route From To distance.pdf" ? "Route London to Liverpool 567km.pdf" And if you had via points "Route London to Liverpool 700km".pdf" I fear we are overthinking and overcoding this :)
Created attachment 323896 [details] [review] printOperation: Change default name of the output file I agree. Looking back now, it's just too much code for something quite minor. Here is a much simpler version, with the starting place, the destination and the distance, and quite reasonable results for most cases.
Review of attachment 323896 [details] [review]: Thanks! ::: src/printOperation.js @@ +53,3 @@ this.onAbortDialogResponse.bind(this)); + let print_settings = new Gtk.PrintSettings(); camelCase! :) printSettings or just settings. @@ +58,3 @@ + this._createFilename(Application.routeService.query.filledPoints, + Application.routeService.route.distance) + ); I do not like this way of breaking arguments, please try to find another way to format this. Maybe use tempVariable for name? @@ +66,3 @@ + _createFilename: function(points, distance) { + let start_place = points[0].place; + let end_place = points[points.length-1].place; weUseCamelCase => startPlace and endPlace @@ +70,3 @@ + if (!start_place.name || !end_place.name) { + let filename = _("Route"); + return filename; just return _("Route"); and no need for braces. @@ +74,3 @@ + + let start_name = start_place.name.split(",")[0]; + let end_name = end_place.name.split(",")[0]; camelCase, but maybe just start and end? @@ +76,3 @@ + let end_name = end_place.name.split(",")[0]; + + let filename = _("{1} to {2} {3}"); Maybe just: return _("%s to %s %s".format(start, end, Utils.prettyDistance(distance, false)); ? (Or did you mean to have prettyDistance round?)
Review of attachment 323896 [details] [review]: Nice work Razvan. ::: src/printOperation.js @@ +69,3 @@ + + if (!start_place.name || !end_place.name) { + let filename = _("Route"); Is this to handle case when we have coordinates instead of locations name? If so, can we use reverse geocoding or will that be too much?
Review of attachment 323896 [details] [review]: I do not think reverse geocoding should be used. 1) There is not guarantee the place that is found will be near enough the point to make sense, we have no control. 2) When we have coordinates it is often because we dragged a route point or used the context menu to find one, meaning we wanted a very exact place to route from. Thanks!
Review of attachment 323896 [details] [review]: ::: src/printOperation.js @@ +58,3 @@ + this._createFilename(Application.routeService.query.filledPoints, + Application.routeService.route.distance) + ); and thinking more, why have The Application... as arguments to createFilename? Just this._crateFilename() would do? And that mehod can access Application.
(In reply to Jonas Danielsson from comment #24) > > and thinking more, why have The Application... as arguments to > createFilename? > Just this._crateFilename() would do? And that mehod can access Application. Thanks, I missed that! I'll take it into consideration before I re-submit de patch. Also, I agree with Jonas, I think reverse geocoding is a bit unstable and should be avoided in this case.
Created attachment 325491 [details] [review] printOperation: Change default name of the output file My PC was dead for the past few days so it took a while for me to get back here. Thanks for the feedback, I think it looks much cleaner now. Also, I think it's ultimately reasonable to use format (instead of my previous version), in order to keep the code more consistent. My hesitation was due to the fact that in some languages the destination might come before the starting point. I have no such examples in my head and I haven't got an answer from the folks at #i18n, so I think it's alright to keep it like this for now.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-maps/issues/48.