Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not use clipboard on X11 #42

Closed
wants to merge 1 commit into from

Conversation

reitzig
Copy link

@reitzig reitzig commented May 28, 2021

Implements #41 for X11.

@reitzig reitzig changed the title fix: do not use clipboard on X11 WIP: fix: do not use clipboard on X11 May 28, 2021
@reitzig
Copy link
Author

reitzig commented May 28, 2021

Maybe wtype could be used for Wayland, but I can't test that.

emote/picker.py Outdated
os.system("xdotool key ctrl+v")
emojis = "".join(self.emoji_append_list)
if not config.is_wayland and self.current_window:
os.system(f"xdotool windowfocus '{self.current_window}' type --args 1 '{emojis}'")
Copy link
Author

Choose a reason for hiding this comment

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

Some remarks.

  • type alone after the sleep didn't work; indeed, the picker window stuck around so I think it received the type.
  • By explicitly targetting the window that was active before the picker came up, we can do away with the sleep.
  • Using windowfocus because type --window doesn't (always) work (neither here nor from the shell); cf. section SENDEVENT NOTES in man xdotool.

@reitzig
Copy link
Author

reitzig commented May 28, 2021

Testing this with "man shrugging":

image

With gedit as target:

image
No emoji font, looks legit.

Thunderbird:
image

Github:
image

Slack:
image
and sometimes
image
and sometimes nothing.

Tweetdeck:
image
But sometimes:
image

Apparently, type sometimes doesn't get through whatever bullshit "modern" web apps do with keyboard events before doing the actually expected thing, at least not fast enough. I never observed this with the clipboard variant.

Maybe a sleep is necessary after all? What's your experience?

I saw some improvement doing this:

os.system(f"xdotool windowfocus --sync '{self.current_window}'")
time.sleep(0.15)
os.system(f"xdotool type --args 1 '{emojis}'")

But the first "character" still got swallowed sometimes. 😪

@reitzig reitzig changed the title WIP: fix: do not use clipboard on X11 fix: do not use clipboard on X11 May 28, 2021
@tom-james-watson
Copy link
Owner

Hello - thanks for your contribution!

I looked into this previously and there's no reliable way to do this on Wayland unfortunately. See atx/wtype#22 for why wtype doesn't work.

I think a better version of what you're trying to do would be to keep the app working the same as it does now, but then to copy back the original clipboard contents after pasting.

However at the moment the app also has some features that are designed around clipboard use. As an example, you can open the app, right click several emojis and then close the app with Esc. The selected emojis are appended to your clipboard as you select them, with the state of which are selected visible in the app. You then would paste those emojis into whatever app you are using. If the app were just selection of single emojis, maybe it would be easier to make this change.

I think if we were to change to restoring clipboard contents after pasting, the app would require some UX tweaks to make it so that the only way to interact with the app is to "finalize" a selection in a way that performs a paste. I would imagine that would also break some flows though where people are relying on having the emojis hang around in their clipboard.

Comment on lines +106 to +111
result = subprocess.run(
['xdotool', 'getactivewindow'],
capture_output = True,
check = True
)
return result.stdout.decode("utf-8").strip()

Choose a reason for hiding this comment

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

Depending on what versions of python are supported, it might be easier to use subprocess.check_output() with text=True.


def add_emoji_to_recent(self, emoji):
user_data.update_recent_emojis(emoji)
emojis.update_recent_category()

Choose a reason for hiding this comment

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

This change should probably be reverted.

@mpsijm
Copy link

mpsijm commented Jan 17, 2023

I just installed Emote from the AUR. It took some manual work to apply the patch, ugly but works 😂 In case this helps anyone, I performed these steps:

  • git clone https://aur.archlinux.org/emote.git
  • git clone https://github.com/mpsijm/Emote.git
  • In Emote: git diff master patch-1 > ../emote/pull-42
  • In emote: apply the below diff to PKGBUILD and makepkg -si
`PKGBUILD` diff
diff --git a/PKGBUILD b/PKGBUILD
index 780b156..79be016 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -14,11 +14,13 @@ source=(
   "https://github.com/tom-james-watson/Emote/archive/${_pkgref}.tar.gz"
   'setup.py'
   'fix-skin-tone-static-path.patch'
+  'pull-42.patch'
 )
 sha512sums=(
   'SKIP'
   'ef8caea8ad9e9bc0487dd8c816561027adda743c1e8e2779a64e7ae99fb227c820f31ef9c87fb910bae7a8ffc623e5e2e1a53a8c69ce0a35ad96557e97a5a949'
   '52d3dce0cecfe62ccc4469b4b35b27c8332d6dce2d1e2dd5603c85a3025fb4a415df303eaaace5579c4204923cbd8d86846deb1026a01f8b201452453c61746f'
+  'SKIP'
 )
 
 build() {
@@ -26,6 +28,8 @@ build() {
   mv -f "$srcdir/setup.py" "$srcdir/Emote-$_pkgref/setup.py"
   # Fix skin tone and `static/` path issues when not running as a snap
   patch -d "$srcdir/Emote-$_pkgref" -p1 <"$srcdir/fix-skin-tone-static-path.patch"
+  # Apply patch from https://github.com/tom-james-watson/Emote/pull/42
+  patch -d "$srcdir/Emote-$_pkgref" -p1 <"$srcdir/pull-42.patch"
   # Move static files into the library
   mv -T "$srcdir/Emote-$_pkgref/static" "$srcdir/Emote-$_pkgref/emote/static"
   # Fix .desktop file



Testing this with "man shrugging":

[...]

Apparently, type sometimes doesn't get through whatever bullshit "modern" web apps do with keyboard events before doing the actually expected thing, at least not fast enough. I never observed this with the clipboard variant.

Maybe a sleep is necessary after all? What's your experience?

I saw some improvement doing this:

os.system(f"xdotool windowfocus --sync '{self.current_window}'")
time.sleep(0.15)
os.system(f"xdotool type --args 1 '{emojis}'")

But the first "character" still got swallowed sometimes. 😪

@reitzig If I split the command and remove the sleep (see mpsijm@patch-1), I'm getting good results in Firefox, Chrome, Telegram, Slack, Gedit, and Xfce4 Terminal 😄

No characters swallowed yet, but then again, I just installed the tool. 😛 I'll report later if I find an app for which it doesn't work. 🙂

EDIT: LibreOffice (tested with Writer and Calc) does seem to swallow characters, but not consistently, with or without sleep 😅

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.

None yet

4 participants