hluk / copyq-commands

Useful commands for CopyQ clipboard manager.
331 stars 73 forks source link

Fix: Uniform data input #33

Closed beefeater7 closed 3 years ago

beefeater7 commented 3 years ago

JSON pasting would create items with unencoded string content. Therefore copy-pasting back and forth would not work, as Copy assumes byte arrays.

hluk commented 3 years ago

Do you have an example of what this fixes?

Strings set as item data are internally converted to UTF-8 encoded ByteArrays. I think the patch shouldn't have any effect if the app work correctly.

hluk commented 3 years ago

Hmm, the check in Copy action data.equals(new ByteArray(text)) seems verify if it's possible to store the data as string in JSON. It would be safer to do this only for text/* formats (and perhaps some internal ones application/x-copyq-*) and otherwise base64-encode the data instead.

beefeater7 commented 3 years ago

Do you have an example of what this fixes?

Copying a pasted item throws the following:

image

...so it's not a two-way transfer. setItem doesn't convert strings into byte arrays. They remain strings until the program is restarted. Maybe it's a program issue but hey, I fixed the script.

beefeater7 commented 3 years ago

Hmm, the check in Copy action data.equals(new ByteArray(text)) seems verify if it's possible to store the data as string in JSON. It would be safer to do this only for text/* formats (and perhaps some internal ones application/x-copyq-*) and otherwise base64-encode the data instead.

I could look into it, but I'm not sure what format would be unsafe to stringify. Please elaborate.

hluk commented 3 years ago

Oh, I noticed the exception when copying from synced tab -- i.e. tab items are synchronized with files on disk. The Synchronize plugin provides additional format application/x-copyq-itemsync-basename which is unfortunately a string (it's not stored). But this is problem with the Copy command and not the Paste one.

I didn't see any problems when copy/pasting content multiple times from in a non-synced tab.

beefeater7 commented 3 years ago

Ok, I disabled all the plugins including sync. Restarted CopyQ to make sure. I still get the error.

Maybe it's a platform thing?

CopyQ Clipboard Manager v3.13.0
Qt: 5.13.2
Compiler: GCC
Arch: i386-little_endian-ilp32
OS: Windows 10 (10.0)

But this is problem with the Copy command and not the Paste one.

I'm not sure I agree. I used to patch over this problem with an extra "text === data" check for Copy. It bit me in the ass for scripting because I would never know if I'm working with byte arrays or strings. It's better to guarantee the consistency of input, I found.

hluk commented 3 years ago

Oh, OK, sorry I misunderstood, because a quick test on my side showed different issue. I thought there was some problem with the item data format being inconsistent -- which is still problem if you copy from synced tab.

Your patch looks good. Thanks.