After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 699118 - window title should match other GNOME applications
window title should match other GNOME applications
Status: RESOLVED FIXED
Product: gitg
Classification: Applications
Component: gui
git master
Other Linux
: Normal normal
: ---
Assigned To: gitg-maint
gitg-maint
Depends on:
Blocks:
 
 
Reported: 2013-04-28 11:22 UTC by Adam Dingle
Modified: 2013-05-03 17:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix gitg window title to match other GNOME applications (1.27 KB, patch)
2013-05-03 14:48 UTC, Sindhu S
needs-work Details | Review
Fix gitg window title to match other GNOME applications (1.21 KB, patch)
2013-05-03 15:29 UTC, Sindhu S
accepted-commit_now Details | Review
Fix gitg window title to match other GNOME applications (1.24 KB, patch)
2013-05-03 16:14 UTC, Sindhu S
needs-work Details | Review
Fix gitg window title to match other GNOME applications (1.20 KB, patch)
2013-05-03 16:29 UTC, Sindhu S
accepted-commit_now Details | Review

Description Adam Dingle 2013-04-28 11:22:33 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
Comment 1 Ignacio Casal Quinteiro (nacho) 2013-04-28 11:49:02 UTC
yeah... I agree on this, I was too lazy when I added the path... :)
Comment 2 Sindhu S 2013-05-03 12:16:31 UTC
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.
Comment 3 Adam Dingle 2013-05-03 12:19:08 UTC
Sindhu,

I think we want to be consistent with gedit.  Can you experiment with gedit and see what it displays in those situations?
Comment 4 Sindhu S 2013-05-03 13:17:27 UTC
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.
Comment 5 Sindhu S 2013-05-03 13:37:38 UTC
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
Comment 6 Adam Dingle 2013-05-03 13:46:48 UTC
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.
Comment 7 Sindhu S 2013-05-03 13:49:12 UTC
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
Comment 8 Sindhu S 2013-05-03 13:50:11 UTC
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.
Comment 9 Sindhu S 2013-05-03 14:48:21 UTC
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.
Comment 10 Adam Dingle 2013-05-03 14:57:35 UTC
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.
Comment 11 Adam Dingle 2013-05-03 14:58:09 UTC
oops - apologies for misspelling your name, Sindhu!  :)
Comment 12 Sindhu S 2013-05-03 15:29:16 UTC
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.
Comment 13 Adam Dingle 2013-05-03 15:41:11 UTC
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!
Comment 14 Sindhu S 2013-05-03 16:14:02 UTC
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! :)
Comment 15 Adam Dingle 2013-05-03 16:22:13 UTC
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.
Comment 16 Sindhu S 2013-05-03 16:29:01 UTC
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).
Comment 17 Adam Dingle 2013-05-03 17:35:19 UTC
Review of attachment 243217 [details] [review]:

Looks good.
Comment 18 Sindhu S 2013-05-03 17:52:44 UTC
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 :)