Merge ~didrocks/ubuntu/+source/gnome-shell:resolve-gdm-symlink into ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/master

Proposed by Didier Roche on 2018-10-22
Status: Merged
Merge reported by: Didier Roche
Merged at revision: 2b1b68eb07d943ba16b9841746f065daa485252c
Proposed branch: ~didrocks/ubuntu/+source/gnome-shell:resolve-gdm-symlink
Merge into: ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/master
Diff against target: 0 lines
Reviewer Review Type Date Requested Status
Ubuntu Desktop 2018-10-22 Pending
Review via email:
To post a comment you must log in.
Marco Trevisan (Treviño) (3v1n0) wrote :

I'd avoid to do the C changes, while you can just use something like this:

try {
    while (GLib.file_test(path, GLib.FileTest.IS_SYMLINK))
        path = GLib.file_read_link(path)
} catch (e) {}

Or even avoiding the file_test check (and replacing it with `true`, since the catch will stop the while anyway).

Another option, just using GFile is instead:

while (true) {
    let info = stylesheet.query_info(Gio.FILE_ATTRIBUTE_STANDARD_IS_SYMLINK+","+
                                     Gio.FileQueryInfoFlags.NONE, null)
    if (!info.get_is_symlink())

    stylesheet = Gio.file_new_for_path(info.get_symlink_target())

So we have less stuff touched and we keep using the glib way.

Also please mention in the patch msg that this could be removed when we've a session refactor as part of this

Marco Trevisan (Treviño) (3v1n0) wrote :

Also, wondering if having this inside st-theme instead of js would make sense, so that once the stylesheet is loaded then we refer resources related to that path.

It's something *maybe* upstreamable. Although probably still not :|

Didier Roche (didrocks) wrote :

Went with the first approach you proposed, which is the more readable IMHO. Thanks!

Preview Diff



People subscribed via source and target branches

to all changes: