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

rofi (custom mode with images) really slow #116

Open
littleblack111 opened this issue Oct 6, 2024 · 14 comments · May be fixed by #124
Open

rofi (custom mode with images) really slow #116

littleblack111 opened this issue Oct 6, 2024 · 14 comments · May be fixed by #124

Comments

@littleblack111
Copy link

its way slower then the normal dmenu one... why.

@littleblack111
Copy link
Author

is it possible to just add the chars for icon to the list command like cliphist list --image, so we don't need to use regex etc to make it slower

@kRHYME7
Copy link

kRHYME7 commented Oct 24, 2024

My guess this one makes it significantly slow
https://github.com/sentriz/cliphist/blob/master/contrib/cliphist-rofi-img

Every call for the clipboard removes the images, then adds it again.

@littleblack111
Copy link
Author

Yes i know that is the one that made it slow(as specified in title) i proposed a solution too. In case anyone wanna implement it to be faster

@kRHYME7
Copy link

kRHYME7 commented Oct 24, 2024

For now, I will be using this.
Added some checks so if file exist then do not create.
1.2s to 0.500s

Having the images handled by cliphist is a great idea

#!/usr/bin/env bash

tmp_dir="/tmp/cliphist"
mkdir -p "$tmp_dir"

if [[ -n "$1" ]]; then
    cliphist decode <<<"$1" | wl-copy
    exit
fi

read -r -d '' prog <<EOF
/^[0-9]+\s<meta http-equiv=/ { next }
/\[ binary/ {
    if (match(\$0, /^([0-9]+)\s(\[\[\s)?binary.*(jpg|jpeg|png|bmp)/, grp)) {
        file_path = "$tmp_dir/" grp[1] "." grp[3]
        if (system("[ -f " file_path " ]") != 0) {
            system("echo " grp[1] "\\\\\t | cliphist decode >" file_path)
        }
        print \$0"\0icon\x1f" file_path
        next
    }
}
1
EOF

cliphist list | gawk "$prog"

image

@kRHYME7
Copy link

kRHYME7 commented Oct 24, 2024

@littleblack111 Something like this ?

image

@littleblack111
Copy link
Author

littleblack111 commented Oct 24, 2024

No. Maybe generate something like the original script, the benefit of this is not needing to pipe it to the program again everytime of an image, since it can be grabbed internally. And prob add an option to put them(like in the script it’s /tmp, maybe default to that)so u can: cliphist -show-image then pipe it to rofi

Sorry im on vacation rn, will get back to u with a more detailed explanation

@kRHYME7
Copy link

kRHYME7 commented Oct 24, 2024

Like this ? @littleblack111
image

@littleblack111
Copy link
Author

Yes that would be it. How’s the performance? Thank you!

@kRHYME7
Copy link

kRHYME7 commented Oct 24, 2024

Not too significant (given the benefit) . And I think the owner of the repo has a better way to optimize this
image

However, I don't really know if this deserves a merge, given that cliphist is clean & minimal.
I would still open a draft pull request though.

@littleblack111
Copy link
Author

Thank you!

@kRHYME7
Copy link

kRHYME7 commented Oct 24, 2024

@littleblack111 BTW, I needed some few suggestions regarding on how we should interface this.

I think it's odd if it only supports rofi. One can write a lightweight clipboard implementation using cliphist with image preview etc.
So I don't want to restrain the feature for rofi only.
so the PR might add 2 or 3 flags.

-image <delimeter string>  export and list the images to the target directory plus add the delimiter string  
-image-dir <target/dir>         optional directory, This is optional as I was thinking screenshots are confidential so they might want to safe keep it somewhere.  default: /tmp/cliphist

Also, performance is improved by just creating the file if not yet existed.
image

@littleblack111
Copy link
Author

Yes that’s true.

That should work.

@kRHYME7
Copy link

kRHYME7 commented Oct 24, 2024

Can you test the patch?

  • But take your time, no rush

@littleblack111
Copy link
Author

littleblack111 commented Oct 25, 2024

yep, this is amazing. had to do a pipe to head -n 100 to achieve the same performance with original script

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 a pull request may close this issue.

2 participants