Skip to content

Conversation

muvment
Copy link
Contributor

@muvment muvment commented Jan 31, 2023

If Linux platform is detected, check if it's using X11 or Wayland server, then execute corresponding copy command.

@eeeXun
Copy link
Owner

eeeXun commented Feb 1, 2023

Great. But I prefer if, else in golang instead of shell. Could you change like this

if os.Getenv("XDG_SESSION_TYPE") == "x11"
...
else
...

Also, please add wl-copy in Dependencies of README

@muvment
Copy link
Contributor Author

muvment commented Feb 2, 2023

For sure! Just realised perhaps I should have opened this as an "enhancement" issue before opening the PR, since I don't know anything about the Go language, my bad.
I'll give it a try though, if I manage I'll update the README too.
If I can't, apologies in advance, and you can close this, I can open it as a suggestion/enhancement in an issue, would that be OK/better?

UPDATE

Ok, so I've reading up and trying things, and I think I got it to work with native go syntax, could you give it a look?
It's working for me on Sway (Wayland)

package main

import (
	"fmt"
	"os"
	"os/exec"
	"runtime"
)

func IndexOf(candidate string, arr []string) int {
	for index, element := range arr {
		if element == candidate {
			return index
		}
	}
	return -1
}

func SetTermTitle(title string) {
	print("\033]0;", title, "\007")
}

func CopyToClipboard(text string) {
	switch runtime.GOOS {
	case "linux":
        if os.Getenv("XDG_SESSION_TYPE") == "x11" {
		exec.Command("sh", "-c",
			fmt.Sprintf("echo -n '%s' | xclip -selection clipboard", text)).
			Start()
        } else if os.Getenv("XDG_SESSION_TYPE") == "wayland" {
                exec.Command("sh", "-c",
			fmt.Sprintf("echo -n '%s' | wl-copy", text)).
			Start()
	}
	case "darwin":
		exec.Command("sh", "-c",
			fmt.Sprintf("echo -n '%s' | pbcopy", text)).
			Start()
	case "windows":
		exec.Command("cmd", "/c",
			fmt.Sprintf("echo %s | clip", text)).
			Start()
	}
}

I added "os" to import and instead of my original pull I defined Wayland as an option too, just in case some other display server exists/might be introduced (although I'm not really aware of any other (major) ones...). I don't know if I could have done it a little cleaner.
Finally, if adding wl-copy to the README I would add it as wl-clipboard, and as "wl-clipboard for Wayland"; should I edit the xclip entry too?
Maybe
"Linux
xclip for X11
wl-clipboard for Wayland"

or

"(Linux) xclip for X11
(Linux) wl-clipboard for Wayland"

What do you think?

@eeeXun
Copy link
Owner

eeeXun commented Feb 2, 2023

Nice, your patch works for me.
But now I think maybe it can use switch-case instead of if-else. It prevent wayland user call os.Getenv("XDG_SESSION_TYPE") this function twice.

Finally, if adding wl-copy to the README I would add it as wl-clipboard, and as "wl-clipboard for Wayland"; should I edit the xclip entry too?
Maybe
"Linux
xclip for X11
wl-clipboard for Wayland"
or
"(Linux) xclip for X11
(Linux) wl-clipboard for Wayland"

I prefer the latter.

@muvment
Copy link
Contributor Author

muvment commented Feb 3, 2023

Great! So you wanna change switch-case for if-else?
Sorry, wouldn't have any clue how to do that. As I said, I have 0 Go language experience 🙁️.
How would you change it to switch-case?
In the mean time I've already modified the README, I'll make a PR once this is fixed.
In the end I kept it more in-tune with your original format and style, so as to not seem out of place:

https://github.com/muvment/GTT/blob/2771c1baa3af0b97ed3affcb427c06c073e1ed43/README.md?plain=1#L72-L78

@eeeXun
Copy link
Owner

eeeXun commented Feb 3, 2023

The switch-case syntax should like

switch os.Getenv("XDG_SESSION_TYPE") {
case "x11":
...
case "wayland":
...
}

In the mean time I've already modified the README, I'll make a PR once this is fixed.
In the end I kept it more in-tune with your original format and style, so as to not seem out of place:

https://github.com/muvment/GTT/blob/2771c1baa3af0b97ed3affcb427c06c073e1ed43/README.md?plain=1#L72-L78

It looks so great.

@muvment
Copy link
Contributor Author

muvment commented Feb 3, 2023

Ok, let me have a look.

@muvment
Copy link
Contributor Author

muvment commented Feb 3, 2023

Something like this?

	case "linux":
	    switch os.Getenv("XDG_SESSION_TYPE") {
        case "x11":
        exec.Command("sh", "-c",
			fmt.Sprintf("echo -n '%s' | xclip -selection clipboard", text)).
			Start()
        case "wayland":
        exec.Command("sh", "-c",
			fmt.Sprintf("echo -n '%s' | wl-copy", text)).
			Start()
        }

edit

It's working for me on Wayland

@eeeXun
Copy link
Owner

eeeXun commented Feb 3, 2023

Yes

@muvment
Copy link
Contributor Author

muvment commented Feb 3, 2023

Great! I'll change it now

If Linux platform is detected, check if it's using X11 or Wayland server, then execute corresponding copy command.
@muvment muvment changed the title Added X11/Wayland detection for copy command Changed "if-else" for "switch-case" Feb 3, 2023
@muvment
Copy link
Contributor Author

muvment commented Feb 3, 2023

Lol, who would have thought it would be that simple, it took me just a minute, I was thinking it would take me another hour or so 😂️.
Anyway, done. If it's good I'll make the PR for the README.

@muvment muvment changed the title Changed "if-else" for "switch-case" @muvment Added X11/Wayland detection for copy command Feb 3, 2023
@muvment muvment changed the title @muvment Added X11/Wayland detection for copy command Added X11/Wayland detection for copy command Feb 3, 2023
@muvment
Copy link
Contributor Author

muvment commented Feb 3, 2023

Btw, how you comment snippets of code with syntax colour highlighting?
And sorry for the title change mistakes, I'm new to this, the UI options confuse me a little still.

@eeeXun eeeXun merged commit 8f2b316 into eeeXun:master Feb 3, 2023
@eeeXun
Copy link
Owner

eeeXun commented Feb 3, 2023

Btw, how you comment snippets of code with syntax colour highlighting?

Add the language to the end of first line backquote, eg. ```go

@muvment muvment deleted the muvment-patch-2 branch February 3, 2023 10:32
@muvment
Copy link
Contributor Author

muvment commented Feb 3, 2023

Cool! Thanks! And thanks for the Go info, I ended up learning some Go along the way. In any case, much appreciated and have a good one 💪️🙏️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants