Created attachment 178456 [details] [review]
places: Remove obsolete code

The dash no longer contains a places section, so remove the now
unused code.

Created attachment 178457 [details] [review]
places: Port to GSettings

'Places' follows the nautilus preference of whether the Desktop
should be a separate directory or the home folder should be used.
Nautilus has been ported to GSettings a while ago, so follow suit.

Created attachment 178458 [details] [review]
places: Fail gracefully when not using unstable nautilus

The latest development version of nautilus has been ported to
GSettings, which we now use as well for the desktop-is-home-dir
preference. Obviously, the required schema is only available if
a recent enough nautilus version is installed. Instead of adding
yet another module to the moduleset, catch the exception and
ignore the preference in case the schema is not available.

Review of attachment 178457 [details] [review]:

Looks fine

Review of attachment 178458 [details] [review]:

::: js/ui/placeDisplay.js
@@ +135,3 @@
+                                             this._updateDesktopMenuVisibility));
+        } catch (e) {
+            log('Failed to get settings from Nautilus. Places may not work as expected');

Can you add a comment here that this is to deal with GNOME 3 nautilus not being installed?

As always, I'm not happy with blanket catches like this, but we don't really have an alternative. :-( Would it be better to do?

 try {
    this._settings = 
 } catch (e) {
    log('');
 }

 if (this._settings != null) {
    [...]
 }

to keep the blanket catch (which could catch all sorts of typos, etc,) as narrow as possible?

Review of attachment 178456 [details] [review]:

Sure

Created attachment 178529 [details] [review]
places: Fail gracefully when not using unstable nautilus

(In reply to comment #5)
> Review of attachment 178458 [details] [review]:
> Can you add a comment here that this is to deal with GNOME 3 nautilus not being
> installed?

OK.


> As always, I'm not happy with blanket catches like this, but we don't really
> have an alternative. :-(

Well, there is the alternative of adding nautilus to the moduleset ... it's just something that looks like complete overkill for a single preference key.


> Would it be better to do?
> 
>  try {
>     this._settings = 
>  } catch (e) {
>     log('');
>  }
> 
>  if (this._settings != null) {
>     [...]
>  }
> 
> to keep the blanket catch (which could catch all sorts of typos, etc,) as
> narrow as possible?

Sure.

Review of attachment 178529 [details] [review]:

Looks good (I meant "alternative to doing catch (e)" ... catching exceptions clearly makes sense in many cases - I just wish we had a better way of catching only the exceptions we wanted to catch)

Attachment 178456 [details] pushed as 89d89ae - places: Remove obsolete code
Attachment 178457 [details] pushed as f91138d - places: Port to GSettings
Attachment 178529 [details] pushed as 26aaecc - places: Fail gracefully when not using unstable nautilus