Created attachment 153735 [details] [review]
Add the workspaces view key to the gconf schema

(In reply to comment #0)
> maybe we should rename the key to something less generic at the same time?

Yes we should rename it to something like "overview_workspace_layout" ... "view" is indeed to generic.

Review of attachment 153735 [details] [review]:

I'd definitely like to see the key renamed as part of this. If short name and the name of the GConf key aren't pretty much the same thing, then something is wrong.

(Would it be better to use strings rather than 0/1? I don't really know what we typically do in GNOME. Think Metacity uses strings for enums. Not a blocker for landing this though if we want to change to strings, renaming the key is a good time to do it and avoid adding compat code.)

Created attachment 154421 [details] [review]
Add the workspaces view key to the gconf schema

renamed the key to overview/workspaces_view and the values to "single"
and "grid". I also changed WorkspacesViewType.MOSAIC to .GRID to match,
because it's not a mosaic... (I didn't change MosaicView though.)

> (Would it be better to use strings rather than 0/1? I don't really know
> what we typically do in GNOME.

gconf prefers strings, and even has helper functions to convert between
enum values and strings. I'm not totally sure I'm happy with the way I
did this here, but it seemed silly to do it by iterating through all the
values comparing numbers and strings like the C helpers do...

Review of attachment 154421 [details] [review]:

Looks good, needs a better commit message that mentions the other changes in the patch.

::: js/ui/workspacesView.js
@@ +959,3 @@
+
+        let view = this._gconf.get_string(WORKSPACES_VIEW_KEY).toUpperCase();
+        if (view in WorkspacesViewType)

Eww :-)

(In reply to comment #4)
> [...] (I didn't change MosaicView though.)

Why not?

didn't feel like making a big invasive change

Attachment 154421 [details] pushed as e5ba414 - Add the workspaces view key to the gconf schema

I'm getting a failure to launch GnomeShell because the GConf client is returning null for the WORKSPACES_VIEW_KEY.

Here is the stack trace:

---
    JS ERROR: !!!   Exception was: TypeError: this._gconf.get_string(WORKSPACES_VIEW_KEY) is null
    JS ERROR: !!!     lineNumber = '970'
    JS ERROR: !!!     fileName = '/home/clown/gnome-shell/source/gnome-shell/js/ui/workspacesView.js'
    JS ERROR: !!!     stack = '()@/home/clown/gnome-shell/source/gnome-shell/js/ui/workspacesView.js:970
WorkspacesViewSwitch()@/home/clown/gnome-shell/source/gnome-shell/js/ui/workspacesView.js:960
()@/home/clown/gnome-shell/source/gnome-shell/js/ui/overview.js:200
Overview()@/home/clown/gnome-shell/source/gnome-shell/js/ui/overview.js:189
start()@/home/clown/gnome-shell/source/gnome-shell/js/ui/main.js:115
@<main>:1
'
    JS ERROR: !!!     message = 'this._gconf.get_string(WORKSPACES_VIEW_KEY) is null'
Window manager warning: Log level 32: Execution of main.js threw exception: TypeError: this._gconf.get_string(WORKSPACES_VIEW_KEY) is null

---

Checking with Configuration Editor reveals no such key at "/desktop/gnome/shell/overview/workspaces_view".

A simple fix might be to set the local variable 'view' to the empty string if GConf returns null, although I'm not an expert on what this code is doing.

the crash is bug 611214; it shouldn't happen if you're using jhbuild

(In reply to comment #9)
> I'm getting a failure to launch GnomeShell because the GConf client is
> returning null for the WORKSPACES_VIEW_KEY.

Try updating your jhbuild checkout (cd ~/Source/jhbuild && git pull -r )

(In reply to comment #10)

Thanks guys.  I am using jbuild.

> Try updating your jhbuild checkout (cd ~/Source/jhbuild && git pull -r )

That did it.