experiment: requests v2

[GeopJr]

sup: #1481
fix: #1480

[bugaevc]

Some quick notes

[bugaevc]

Suggested change

	if (this.cancellable != null && !this.cancellable.is_cancelled ()) this.cancellable.cancel ();
	this.cancellable?.cancel ();

GLib.Cancellable.cancel () already performs the already-cancelled check.

[GeopJr]

Removed the is_cancelled check, but fwiw, I'm hesitant on using optional chaining in vala due to #1002 and f5bd1c9 (couldn't reliably reproduce to report upstream)

[GeopJr]

Thanks for the review!

Maybe the API should change again and return the inputstream instead of outing it, now that we don't need to return bool

[bugaevc]

Yeah that makes sense too.

[GeopJr]

Moving forward with replacing all instances, something that caught my attention is vala's construction order:

// Home.vala, moving the refreshing away from idle and to just the async function
	construct {
		url = "/api/v1/timelines/home";
		label = _("Home");
		icon = "tuba-user-home-symbolic";

fails with url == null

but

    public Home () {
        Object (
            url: "/api/v1/timelines/home",
	    label: _("Home")
        );
    }

works. At the same, the Notifications.vala url depends on the account, which is not available during Notifications () {, but leaving it as is doesn't cause any url == null

🤔 Personally, I'd expect construct to run before using the Object at all. I know I'm wrong now, but I thought it was like this: construct -> Home () -> Object ()

[GeopJr]

More annoyances:

public Views.ContentBase add_timeline_tab (string label, string icon, string url, Type accepts, string? empty_state_title = null, string? empty_state_icon = null) {
		var tab = new Views.Accounts () {
			url = url,
			label = label,
			icon = icon,
			accepts = accepts
		};

url == null, but introducing this works:

public class Tuba.Views.Accounts : Views.Timeline {
	public Accounts.prefilled (string url, string label, string icon) {
		Object (
			url: url,
			label: label,
			icon: icon
		);
	}

[bugaevc]

On top of GObject's already complicated construction/initialization flow, Vala adds its own layer of confusion 😭 I hate everything about the way it's done, and it's unlikely that I could change much here about Vala semantics (especially given the state of Vala maintenance, which makes it hard to get any non-trivial change merged into Vala).

You would think new Type () { prop1= value1, prop2 = value2 }; syntax calls g_object_new (THE_TYPE, "prop1", value1, "prop2", value2, NULL), right? But it doesn't, instead it calls the regular constructor (the_type_new ()) followed by setting the individual properties.

Properties whose values are visible at constsruct { } time are the ones that both:

• are "construct properties", i.e. marked construct or construct set
• were passed to the g_object_new call, i.e. Object () chain-up in Vala

What you mean about Home.vala and URL being null, I couldn't really follow (more details please?), but possibly you're reading the value of the url property (or starting an async operation which does just that) from the construct block of a parent class, but setting that property from the construct block of a derived class? The derived class's construct runs later than the parent's, hence the value is not yet set at that point — if that's what you're doing & running into.

From the GObject standpoint, the right thing to do is:

1. Passing as many properties as possible into the g_object_new call, as opposed to creating an instance first and setting properties later — i.e. the opposite of what new Type () { prop = value } syntax does in Vala. You could achieve this in Vala by calling g_object_new more explicitly (Object.@new (typeof (Type), prop: value)), but this is somewhat ugly; alternatively, creating special-case constructors with Object () chain-ups, as in your Accounts.prefilled example — also cumbersome.
2. Generally, doing/starting various things in property setters, when a value is set for a property, and not "upon creation". E.g. I imagine a request could be started from the url property's setter. This way, if the object does get created without values for all these properties, those "various things" do happen when the values do get set. It's also a good opportunity to make the property re-settable, i.e. start the setter by undoing whatever setup it did for the old value (e.g. cancelling an old request, if any, when the URL changes). For example, see how Gtk.DirectoryList reacts when its file property is set (gtk_directory_list_set_file).
3. When doing things in setters is not feasible, marking properties construct and doing things in the constructed vfunc; Vala's construct blocks actually translate to the constructor vfunc, which is worse, but close enough.

Here's a piece of code for you to play with:

#! /usr/bin/vala

public class Base : Object {
	public int some_prop {
		set { print ("Setting some_prop to %d\n", value); }
	}
	public int some_construct_prop {
		construct set { print ("Setting some_construct_prop to %d\n", value); }
	}

	public Base () {
		print ("Base: before chain-up\n");
		Object (some_prop: 42);
		print ("Base: after chain-up\n");
	}

	construct {
		print ("Base: construct block\n");
	}
}

public class Derived : Base {
	public Derived () {
		print ("Derived: before chain-up\n");
		base ();
		print ("Derived: after chain-up\n");
	}

	construct {
		print ("Derived: constrct block\n");
	}
}

void main () {
	new Derived ();
}

I get:

Derived: before chain-up
Base: before chain-up
Setting some_construct_prop to 0
Base: construct block
Derived: constrct block
Setting some_prop to 42
Base: after chain-up
Derived: after chain-up