SoMuchForSubtlety / f1viewer

🏎️ TUI for F1TV
GNU General Public License v3.0
758 stars 65 forks source link

Add support for flatpak versions of MPV and VLC #188

Closed hkgranli closed 3 years ago

hkgranli commented 3 years ago

This change adds detection and execution support for Flatpak versions of mpv and VLC on Linux.

example

SoMuchForSubtlety commented 3 years ago

Thank you for the pull request!

I looked over your code and I think we can accomplish working with flatpaks in a simpler way. Instead of flatpak being a boolean, make it the flatpak app ID, then we can use a similar mechanism we use for the windows registry. Roughly like this

    for _, c := range commands {
        _, err := exec.LookPath(c.Command[0])
        if err == nil {
            store.Commands = append(store.Commands, c)
        } else if c, found := checkRegistry(c); found {
            store.Commands = append(store.Commands, c)
        } else if c, found := checkFlatpak(c); found {
            store.Commands = append(store.Commands, c)
        }
    }

Then add checkFlatpak to registry.go and registry_unix.go (we should probably also rename them to something like lookup.go)

checkFlatpak will check if the user has the flatpak installed, and if they do, they can edit the command to use the flatpak. Something like

func checkFlatpak(c Command) (Command, bool) {
    if c.flatpakAppID == "" {
        // no flatpak app ID specified
        return c, false
    }
    _, err := exec.LookPath("flatpak")
    if err != nil {
        // flatpak not installed
        return c, false
    }
    err := exec.Command("flatpak", "info", c.flatpakAppID).Run()
    if err != nil {
        // not installed
        return c, false
    }

    c.Command[0] = c.flatpakAppID
    c.Command = append([]string{"flatpak", "run"}, c.Command...)

    return c, true
}

What do you think?

hkgranli commented 3 years ago

Your solution was definitely cleaner than mine, I added the method with a small title change to registry_unix:

func checkFlatpak(c Command) (Command, bool) {
    if c.flatpakAppID == "" {
        // command is not flatpak
        return c, false
    }

    _, err := exec.LookPath("flatpak")
    if err != nil {
        // flatpak not installed
        return c, false
    }

    err = exec.Command("flatpak", "info", c.flatpakAppID).Run()
    if err != nil {
        // package not installed
        return c, false
    }

    c.Command[0] = c.flatpakAppID
    c.Command = append([]string{"flatpak", "run"}, c.Command...)

    // optional update title

    c.Title = c.Title + " Flatpak"

    return c, true
}

As for registry.go I assumed it would only be called if the program is being ran on a Windows system? In which case the return value would always be false.