gnome-shell-extension-gsconnect icon indicating copy to clipboard operation
gnome-shell-extension-gsconnect copied to clipboard

Cli support for general wayland environment

Open JingMatrix opened this issue 2 years ago • 3 comments

This commit use wl-clipboard and ydotool to support general wayland environment, such as sway.

JingMatrix avatar Apr 02 '22 14:04 JingMatrix

Please tell me if you have intention to extend this plugin to non-gnome environment, @ferdnyc @andyholmes . If so, I can add a page to the project wiki to show how it actually works.

JingMatrix avatar Apr 08 '22 07:04 JingMatrix

I don't have much of an opinion on this. GSConnect was intended to be specifically for GNOME Shell, but if another maintainer wants to give it a :+1: I won't stand in the way :slightly_smiling_face:

andyholmes avatar Apr 08 '22 20:04 andyholmes

I have limited time for maintenance, so if making it more generic comes with associated maintenance effort on an ongoing basis I'm all for it. I'll read through the code hopefully soon.

daniellandau avatar May 22 '22 09:05 daniellandau

Oh, just one more thing: add the SPDX comments to ydotool.js and wl_clipboard.js like so:

// SPDX-FileCopyrightText: GSConnect Developers https://github.com/GSConnect
//
// SPDX-License-Identifier: GPL-2.0-or-later

andyholmes avatar Jun 15 '23 18:06 andyholmes

Thanks for reviewing. Unfortunately, I have forgotten most of the logic of gsconnect, though I am using this pull request for the whole year. It seems after merging the main branch, remote mousepad isn't working. I need some time to debug it. Also, I should add a wiki page to explain the usage.

JingMatrix avatar Jun 15 '23 21:06 JingMatrix

Okay, no problem. I don't expect many folks will start using this for a little while, at least.

If you can get it working for you, we can merge and you can be "codeowner" for that module, if you like.

andyholmes avatar Jun 15 '23 23:06 andyholmes

I test it on my machine with both sway and gnome-shell sessions, and there is no problem. (It turned out that I forgot to start ydotoold in sway.) I would be happy to maintain this module.

Notes for users

When gsconnect is not running as a Gnome extension, for example, running under sway,

  1. You might still need to install the gnome-shell package to provide the Gvc-1.0.typelib file of the gnome module libgnome-volume-control.
  2. Install ydotool and start the service: systemctl status --user ydotool.service
  3. Install wl-clipboard and wl-type

JingMatrix avatar Jun 15 '23 23:06 JingMatrix

I found that the env GNOME_SETUP_DISPLAY is specially set by gnome, which can thus be used to detect gnome.

Now this pull request should pass all tests without problems, you can merge it.

P.S. it might be interesting to write a test for non-gnome envirment, maybe in the future.

JingMatrix avatar Jun 16 '23 00:06 JingMatrix

P.S. it might be interesting to write a test for non-gnome envirment, maybe in the future.

Definitely, I saw somewhere I think Google Chrome or Chromium had a Wayland version of xvfb built with python or something. I never had time to really get into it though.

andyholmes avatar Jun 16 '23 04:06 andyholmes

As a note, I add the wiki page of its usage: CLI usage without Gnome.

It will be helpful to put some instructions described there into the meson build process.

JingMatrix avatar Jun 16 '23 09:06 JingMatrix

That's great, thanks for writing all that up!

andyholmes avatar Jun 17 '23 21:06 andyholmes

This PR introduces a check that uses imports to determine, as far as I understand, if it's running inside gnome shell. What was the problem with that? And when it was replaced, why was it replaced with a check for the GNOME_SETUP_DISPLAY environmental variable? Why wasn't something like GLib.getenv('XDG_CURRENT_DESKTOP') === 'GNOME' || GLib.getenv('XDG_SESSION_DESKTOP') === 'gnome' enough? The only clue I could find is in 9739f97c5276809de297768e628578f134409261 that states

Relying on environment variables for the determination of gnome-shell is not stable when run as a d-bus service.

So I'm guessing the concern is that checks like the above will break gsconnect if it's run inside a gnome session but not as a gnome shell extension? Because if so, then I believe the current check has the same weakness.

As I mentioned in https://github.com/GSConnect/gnome-shell-extension-gsconnect/issues/1706#issuecomment-1813355435 now that gsconnect checks GNOME_SETUP_DISPLAY, some features are broken on all configurations that have no ydotool, and running x11 or running wayland without xwayland.

pobrn avatar Dec 04 '23 22:12 pobrn

Given what said by @pobrn , it is better not to use environment variable for detecting the 'Gnome'. I was for using imports to detect Gnome, @andyholmes changed it out of the reason of simplicity (I guess). Hence, it is favorable to revert the changes regarding the environment variable. I will do it soon to fix #1706.

JingMatrix avatar Dec 05 '23 10:12 JingMatrix

FWIW, this was my suggestion:

This won't work in the service, because it's runs outside of the gnome-shell process (meaning it will always throw an error). However, I think this would work:

const HAVE_GNOME = GLib.getenv('XDG_SESSION_TYPE') === 'gnome'

You can put this in src/service/__init__.js as globalThis.HAVE_GNOME if you want, or leave it here.

andyholmes avatar Dec 05 '23 15:12 andyholmes

@andyholmes Thanks for mentioning it, I totally forgot it. So I guess we may use the existence of the directory /usr/share/cinnamon/js/ui/* as a detector.

Or, alternatively, is there a way to communicate bewteen the extention.js and daemon.js? If so, we can set HAVE_GNOME in extension.js.

JingMatrix avatar Dec 05 '23 16:12 JingMatrix

I propose to use imports.gi.Shell as a heuristic for setting HAVE_GNOME. See my pull-request for details: https://github.com/GSConnect/gnome-shell-extension-gsconnect/pull/1715#issuecomment-1841296211

JingMatrix avatar Dec 05 '23 17:12 JingMatrix