Created attachment 188705 [details] [review]
Use user-defined calendar application for the date menu calendar button

Use the existing setting

  org.gnome.desktop.default-applications.office.calendar.exec

as calendar application instead of the hard-coded evolution.  Evolution
is still the fallback if that setting is cleared (it defaults to
evolution).

Comment on attachment 188705 [details] [review]
Use user-defined calendar application for the date menu calendar button

>+        let calendarSettings = new Gio.Settings({ schema: 'org.gnome.desktop.default-applications.office.calendar' });
>+	let tool = calendarSettings.get_string('exec');

watch out on spaces-vs-tabs. There should be no tabs in JS files.

>+	// Fall back to evolution (and call the right day)
>+	if (tool.length == 0 || tool == 'evolution') {
>+	    // TODO: pass the selected day
>+            Util.spawn(['evolution', '-c', 'calendar']);

arguably the schema is broken here and ought to default to "evolution -c calendar"...

>+	} else {
>+	    let toolArray = tool.split(' ');
>+	    let terminal = calendarSettings.get_boolean('needs-term');
>+	    if (terminal) {
>+		Util.spawn(['gnome-terminal', '-e'].concat(toolArray));
>+	    } else {
>+		Util.spawn(toolArray);

this is wrong in several ways. First, you can't just split on spaces, because there may be quoted strings in the command. But second, you don't want to split it into argv anyway, because you have to pass the command as a single argument to gnome-terminal. So you want to use Util.spawnCommandLine here.

However... it should also use org.gnome.desktop.default-applications.terminal rather than assuming gnome-terminal.

> arguably the schema is broken here and ought to default to "evolution -c
> calendar"...

Well, not my fault.  I kept that statement as-is. :-)

With regard to the other problems (TABs, usage of default terminal, spawning commands correctly) I've created a new version of the patch.

Thanks a ton of the comments. :-)

Tassilo

Created attachment 190843 [details] [review]
Uptated patch

Comment on attachment 190843 [details] [review]
Uptated patch

>+                Util.spawnCommandLine(term.concat(' ').concat(arg)
>+                                      .concat(' ').concat(tool));

ew. you can just do: term + ' ' + arg + ' ' + tool

however, I was wrong before, you want to do

    if (arg != '')
        Util.spawn([term, arg, tool]);
    else
        Util.spawn([term, tool]);

do you have a GNOME git account or do you need someone to commit this for you?

Hi Dan,

sorry for responding so late, I've been on a holiday trip. :-)

Here's an updated patch, and no, I don't have a gnome git account, so someone has to commit for me.

Created attachment 193616 [details] [review]
Updated patch (2)

I split the tabs-vs-spaces fixes out into a separate patch and then
committed. Thanks for the patches.