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 699146 - display error when opening directory without a git repository
display error when opening directory without a git repository
Status: RESOLVED FIXED
Product: gitg
Classification: Applications
Component: gui
git master
Other Linux
: Normal normal
: ---
Assigned To: Sindhu S
gitg-maint
: 697938 699233 (view as bug list)
Depends on:
Blocks: 702666
 
 
Reported: 2013-04-28 15:05 UTC by Adam Dingle
Modified: 2013-07-06 05:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add infobar (5.79 KB, patch)
2013-06-20 18:59 UTC, Sindhu S
none Details | Review
Add infobar (5.75 KB, patch)
2013-06-21 18:56 UTC, Sindhu S
none Details | Review
Add infobar (6.18 KB, patch)
2013-06-22 13:24 UTC, Sindhu S
needs-work Details | Review
Add infobar (5.98 KB, patch)
2013-06-22 17:09 UTC, Sindhu S
accepted-commit_now Details | Review

Description Adam Dingle 2013-04-28 15:05:49 UTC
To see the problem, choose Open Repository, then choose a directory which does not contain a git repository.  The operation fails silently.

Instead, gitg should display an error message such as "The directory <path> does not contain a git repository."
Comment 1 Sindhu S 2013-04-29 17:55:19 UTC
Thanks for the bug report. This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find.

*** This bug has been marked as a duplicate of bug 699233 ***
Comment 2 Sindhu S 2013-04-29 17:58:16 UTC
Oh damn, I forgot to check the timestamps before marking this bug as resolved and duplicate of the one I filed! Sorry again, Adam :(

The correct choice was to close the bug I filed as duplicate of this bug.
Comment 3 Adam Dingle 2013-04-29 18:50:28 UTC
Yes, usually the right thing to do is to close the newer bug as a duplicate of the older one.   :)
Comment 4 Adam Dingle 2013-04-29 18:52:28 UTC
And by the way, you don't need to compare timestamps - just compare the bug numbers.  :)
Comment 5 Sindhu S 2013-04-29 18:53:41 UTC
*** Bug 699233 has been marked as a duplicate of this bug. ***
Comment 6 Sindhu S 2013-05-03 16:46:03 UTC
*** Bug 697938 has been marked as a duplicate of this bug. ***
Comment 7 Sindhu S 2013-06-20 18:59:28 UTC
Created attachment 247380 [details] [review]
Add infobar

Please review, based on this I can route currently silent errors to infobar and solve bug 699153, bug 699146.
Comment 8 Paolo Borelli 2013-06-21 18:45:48 UTC
Review of attachment 247380 [details] [review]:

The patch looks good to me, but I had no chance to test it yet

Just a tiny nitpick below

::: gitg/gitg-window.vala
@@ -207,2 +216,3 @@
 				}
 				title = @"$(d_repository.name) ($parent_path) - gitg";
+				if (infobar.visible)

no need to check if it is visible, just call hide(), if it is already hidden it is a no-op
Comment 9 Sindhu S 2013-06-21 18:56:32 UTC
Created attachment 247481 [details] [review]
Add infobar

Updated patch, please review. Thank you!
Comment 10 jessevdk@gmail.com 2013-06-22 07:52:28 UTC
Review of attachment 247481 [details] [review]:

::: gitg/gitg-window.vala
@@ +697,3 @@
+		infobar.message_type = type;
+		infobar_label_primary.set_label ("<b>%s</b>".printf (primary_msg));
+		infobar_label_secondary.set_label ("<small>%s</small>".printf (secondary_msg));

Just a minor detail here, but maybe we should escape the primary and secondary message. Otherwise the caller needs to make sure they are properly escaped.
Comment 11 Sindhu S 2013-06-22 13:24:37 UTC
Created attachment 247513 [details] [review]
Add infobar

Updated patch, please review. Thanks.
Comment 12 Ignacio Casal Quinteiro (nacho) 2013-06-22 16:01:45 UTC
Review of attachment 247513 [details] [review]:

Check the inline comments. Also I would like to have the infobar inside a revealer.

::: gitg/gitg-window.vala
@@ +144,2 @@
 		d_main_settings = new Settings("org.gnome.gitg.preferences.view.main");
+		d_interface_settings = new Settings("orgf.gnome.gitg.preferences.interface");

why this change? a typo by mistake?

::: gitg/resources/ui/gitg-window.ui
@@ +134,3 @@
+                 <property name="spacing">16</property>
+                 <child>
+                   <object class="GtkLabel" id="infobar_label_primary">

infobar_primary_label

@@ +148,3 @@
+                 </child>
+                 <child>
+                   <object class="GtkLabel" id="infobar_label_secondary">

infobar_secondary_label

@@ +170,3 @@
+                     <property name="layout_style">end</property>
+                 <child>
+                   <object class="GtkButton" id="infobar_button_close">

infobar_close_button
Comment 13 Sindhu S 2013-06-22 17:09:09 UTC
Created attachment 247522 [details] [review]
Add infobar

Updated patch.
Comment 14 Ignacio Casal Quinteiro (nacho) 2013-06-23 08:55:48 UTC
Review of attachment 247522 [details] [review]:

Looks good to me. In a second patch I'd like you to modify it to use a GtkRevealer similar to what we do in gedit-tab.c
Comment 15 Sindhu S 2013-06-23 10:54:01 UTC
(In reply to comment #14)
> Review of attachment 247522 [details] [review]:
> 
> Looks good to me. In a second patch I'd like you to modify it to use a
> GtkRevealer similar to what we do in gedit-tab.c

Committed to master in a5972bd
Available at https://git.gnome.org/browse/gitg/commit/?id=a5972bd72c9f7ce495bdbc0c61f0e9262c519afe

Leaving bug open for the next iteration of the patch that will make Infobar use GtkRevealer.
Comment 16 Sindhu S 2013-07-06 05:31:47 UTC
Infobar was made to use GtkRevealer in commit b8fa527.
Available at
https://git.gnome.org/browse/gitg/commit/?id=b8fa527842bb7c53b0140577a33281a5b9ed3d30

Closing bug with status 'RESOLVED' and 'FIXED', as all requirements seem have to been satisfied.

Thank you.