Skip to content
This repository has been archived by the owner on Feb 10, 2023. It is now read-only.

Add support for flatpak versions of MPV and VLC #188

Merged
merged 7 commits into from
Oct 13, 2021
Merged

Add support for flatpak versions of MPV and VLC #188

merged 7 commits into from
Oct 13, 2021

Conversation

hkgranli
Copy link
Contributor

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

example

Added support for official flatpak versions of VLC and mpv
@SoMuchForSubtlety
Copy link
Owner

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
Copy link
Contributor Author

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.

Copy link
Owner

@SoMuchForSubtlety SoMuchForSubtlety left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added two comments. Please also resolve your conflicts / rebase your branch.

internal/cmd/registry_unix.go Outdated Show resolved Hide resolved
internal/cmd/registry.go Outdated Show resolved Hide resolved
Copy link
Owner

@SoMuchForSubtlety SoMuchForSubtlety left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution :)

@SoMuchForSubtlety SoMuchForSubtlety merged commit afb883f into SoMuchForSubtlety:master Oct 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants