GNOME Bugzilla – Bug 699118
window title should match other GNOME applications
Last modified: 2013-05-03 17:52:44 UTC
I'm running gitg built from git master. When I'm viewing a repository in gitg, currently the title bar looks like this: (/home/adam/src/seahorse) - gitg Instead, I think it should look like this to match other GNOME applications such as gedit: seahorse (~/src) - gitg
yeah... I agree on this, I was too lazy when I added the path... :)
Hello, I have already have a patch that makes requisite changes for this bug: title = "(%s) - gitg".printf(workdir.get_parent().get_relative_path(workdir)); This will print title of the window to be the directory name of the repository only. However, I want to confirm on how I should go ahead with these details: 1. If the repository belongs to a $USER's $HOME directory, then: Would you like "~/../../repository" displayed? 2. If the repository belongs to / (example: I open repositories from a mounted directory called /code on my vagrant setup) then: Would you like "/../../../repository" displayed? Here, the ../indicate the depth of the directory. Please confirm, thank you.
Sindhu, I think we want to be consistent with gedit. Can you experiment with gedit and see what it displays in those situations?
I am reporting these title styles by running Gedit that was recently compiled from Git master: 1. If a file originates from user's $HOME directory, then the window title format is: <filename.ext> (~/directory1/.../directoryN) - Program_Name 2. If a file originates from / (that is, root), then the window title format is: <filename.ext> (/directory1/.../directoryN) - Program_Name So I believe that the most ideal of solving this bug would be to: Step 1: Get Environment.get_home_dir() Step 2: Compare if this Step 1 string exists in the starting of somefileobject.get_parent().get_path(); Step 3: If yes, precede title with ~ Step 4: If no, precede title with / Step 5: Calculate depth of directories (that is, how many directories will it take to `cd` out of the current directory back to ~) and depending on integer value returned, insert that many "../" between prefix (~ or /) and relative path. Step 6: Append "Gitg" to this constructed title. Note: In any other program if there was capability of browsing directories (that is, navigating into and out of directories) then the title must be reconstructed as the navigation would change the depth of the directories, however, in Gitg the title need not be reconstructed because navigation of directories is not possible. When ever a repository is opened, the title will be regenerated for the Gitg window and this is sufficient. Please confirm.
I got confused the first time around. Calculating depth of directories won't be necessary at all. As gedit prints full path without replacing them with ../ I have to just print in the format: repository name (full path preceding with either ~ or / depending on it's origin directory of course) and append with " - Gitg". example1: seahorse (/code/seahorse) - Gitg example2: searhorse (~/code/seahorse) - Gitg
That's right. Actually I think it will look like this: example1: seahorse (/code) - gitg example2: seahorse (~/code) - gitg Note that the program name is actually "gitg" in all lowercase.
Experimenting some more I realised the format should be: repository-name (~/full/path/upto/parent/directory) - Program name Considering the repository is located at code/mycode/src/seahorse Example: Seahorse (~/code/mycode/src/) - Gitg
Adam, looks like we were posting the comment at the same time! (I got a mid-air collision notice) anyway, I am making the patch now, thanks.
Created attachment 243189 [details] [review] Fix gitg window title to match other GNOME applications Gitg windows now show title of the format: repository-name (preceding with ~ if from $USER directory or /path/to/parent/directory) - program-name Please review, thank you.
Review of attachment 243189 [details] [review]: Sindha, this is looking pretty good. My only comment is that you're introducing duplicated code in both branches of your if/else statement; they both contain title = "%s (%s) - gitg".printf(d_repository.name, ... Please see if you can factor this out so it only appears once.
oops - apologies for misspelling your name, Sindhu! :)
Created attachment 243202 [details] [review] Fix gitg window title to match other GNOME applications Adam, No problem :) I understand typos, am a frequent victim myself! Updated patch, refactored if statement. Please review, again. Thanks.
Review of attachment 243202 [details] [review]: Sindhu, this looks good. - You need a space inside "parent_path=". - By the way, instead of the printf you could use a Vala string template - see https://live.gnome.org/Vala/Tutorial#Strings and https://live.gnome.org/Vala/StringSample. Or the printf is fine if you prefer that - it's up to you. No need for another review iteration unless you want one. Thanks!
Created attachment 243211 [details] [review] Fix gitg window title to match other GNOME applications I made another patch which uses string templates :) Though, I had to create another string variable because I noticed that String templates can't parse properties (example: d_repository.name), compiler complains throwing an error. I'll commit whichever you or Paolo or Iganacio or Jesse asks me to commit to master. Thank you for the very patient reviews! :)
Review of attachment 243211 [details] [review]: Sindhu, string templates *can* contain expressions; you just need to surround them with parentheses, like this: "... $(d_repository.name) ..." So if you'd like to use a string template, the extra variable is unnecessary.
Created attachment 243217 [details] [review] Fix gitg window title to match other GNOME applications Made changes as per previous review for this patch (with String templates).
Review of attachment 243217 [details] [review]: Looks good.
Pushed to master in commit 5104c62bc1adfa6f1c5cd7afbc82dfbac23f4820 Available at https://git.gnome.org/browse/gitg/commit/?id=5104c62bc1adfa6f1c5cd7afbc82dfbac23f4820 Closing bug by marking as RESOLVED and FIXED now. Thank you, Adam :)