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

[WIP] Copy-paste patch objects using system clipboard text buffer #2086

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

eeropic
Copy link

@eeropic eeropic commented Sep 5, 2023

Opening wip PR for discussion
Related issue #635

Todo

  • do not use static variables (use gl_privatedata for clipboard length and text)
  • edit commit history so copy/paste functionality are in separate commits, remove unnecessary
  • refactor, DRY, etc..
  • copy: test serialization (escaping special chars in particular) by diffing saved patches and patches serialized to clipboard
  • memory allocation checks (+error handling)
  • Validate patch data before attempting paste (regex?)
  • handle atom width attribute, i.e. ,f 16 in the end of each patch line (atom width in chars)

Maybe

  • figure out whether to include #N for new canvas. i.e. on copying, should the copied patch include the canvas in order to be saved as a proper patch, and on pasting, the first #N should be then ignored.
  • Should paste be single action instead of two? Currently there's 2 functional copy buffers (internal and text)

Testing

  • Test on windows (latest build tested on win10 64bit)
  • Test on osx (latest build tested on arm64 m1, osx 13.2.1)
  • Test on linux (latest build tested on arm64 debian 11)

Other

  • escape clipboard string when sending back from tcl to c when pasting
  • Text to binbuf chunks - maximum string length is now limited by MAXPDSTRING, which limits the amount of objects you can copy/paste
  • split lines to handle longer messages (long arrays for example) through pdsend
  • bug: handle double paste on mac, resulting from menu command keybind declaration running the command twice
  • handle paste undo

Copy link
Contributor

@umlaeute umlaeute left a comment

Choose a reason for hiding this comment

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

pdgui_vmess already takes care of escaping.
just use it.

src/g_editor.c Outdated Show resolved Hide resolved
@eeropic eeropic force-pushed the copy-paste-clipboard-text-buffer branch 2 times, most recently from 7060e56 to f41f9e2 Compare September 5, 2023 19:45
@eeropic eeropic marked this pull request as draft September 5, 2023 19:47
@eeropic eeropic force-pushed the copy-paste-clipboard-text-buffer branch from f41f9e2 to 6b1c7f8 Compare September 5, 2023 19:51
Copy link
Contributor

@umlaeute umlaeute left a comment

Choose a reason for hiding this comment

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

better but not there yet

src/g_editor.c Outdated Show resolved Hide resolved
bind_capslock all $::modifier-Key a {menu_send %W selectall}
bind_capslock all $::modifier-Key b {menu_helpbrowser}
bind_capslock all $::modifier-Key c {menu_send %W copy}
bind_capslock all $::modifier-$alt-Key c {menu_send %W copy-to-clipboard-as-text}
Copy link
Contributor

@umlaeute umlaeute Sep 5, 2023

Choose a reason for hiding this comment

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

the Alt/Option is also handled in ::pd_bindings::global_bindings, i think it should be handled only once (like with $::modifier)

tcl/pd_menus.tcl Outdated
} else {
set accelerator "Ctrl"
set alt "Alt"
Copy link
Contributor

Choose a reason for hiding this comment

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

like this.

@eeropic
Copy link
Author

eeropic commented Sep 6, 2023

Also still need to properly escape brackets, commas, etc when sending back from Tcl.

Im looking at

proc unspace_text {x} {
and pdtk_text.tcl, but not sure if theres already a proc that does this non-destructive (i.e. not removing commas altogether)

canvas_copy_to_clipboard_as_text() performs canvas_docopy and gets the binary object buffer as text, and passes it to TCL to copy to system clipboard

canvas_paste_from_clipboard_text() calls TCL to send system clipboard text, tcl then calls canvas_got_clipboard_content()
which transforms the text back to binary buffer and performs canvas_dopaste

TODO: split clipboard append to chunks
@eeropic eeropic force-pushed the copy-paste-clipboard-text-buffer branch from 2d3492a to de783d1 Compare September 8, 2023 06:29
src/g_editor.c Outdated Show resolved Hide resolved
src/g_editor.c Outdated
{
pdgui_vmess("pdtk_get_clipboard_text", "^", x);
}
static t_binbuf *clipboard_patch_bb = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

this should go into (t_glist*)->gl_privatedata

src/g_editor.c Outdated Show resolved Hide resolved
src/g_editor.c Outdated Show resolved Hide resolved
@umlaeute
Copy link
Contributor

i think i discovered a bug in pdgui_vmess() where A_COMMA and a comma.symbol ( gensym(",")) are both sent as ,.

this needs to be fixed before this PR can properly handle appendicles (, f ...)

@eeropic
Copy link
Author

eeropic commented Sep 26, 2023

i think i discovered a bug in pdgui_vmess() where A_COMMA and a comma.symbol ( gensym(",")) are both sent as ,.

this needs to be fixed before this PR can properly handle appendicles (, f ...)

Ok, I was wondering about this before.
Now tried printing out on save, and on copy, and can verify.
Saving: (printing binbuf before free at canvas_savetofile)

#X
text
34
25
look
\,
setting
width
,
f
12
;

Copying (pdgui_vmess()):

#X
text
34
25
look
,
setting
width
,
f
12
;

@eeropic
Copy link
Author

eeropic commented Sep 27, 2023

i think i discovered a bug in pdgui_vmess() where A_COMMA and a comma.symbol ( gensym(",")) are both sent as ,.

this needs to be fixed before this PR can properly handle appendicles (, f ...)

Even if I have the , f ... correctly in text clipboard, it needs to be handled separately (thinking of another flag) as pdsend will split the message / interpret it as 2 messages delimited by comma

src/g_editor.c Outdated
case CLIPBOARD_PATCH_TEXT_LINE_PARTIAL:
case CLIPBOARD_PATCH_TEXT_LINE_END:
if (clipboard_patch_bb) {
case CLIPBOARD_PATCH_TEXT_LINE_END_APPENDIX:
Copy link
Contributor

@umlaeute umlaeute Sep 28, 2023

Choose a reason for hiding this comment

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

i'm not exactly sure about this.
since a Pd-file can be properly parsed by binbuf_text(), even if it contains message delimiting commas ([foo, bar() and gobj attributes (, f 10), I do not see why need special handling here.
ideally, the canvas_got_clipboard_contents() should get the raw text (as in the clipboard), and that's it (and all the necessary escaping happens in the GUI)

#X msg 78 108 foo \, bar, f 30;

Copy link
Contributor

Choose a reason for hiding this comment

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

(and sorry for not saying that before you started to implement it)

Copy link
Author

@eeropic eeropic Sep 28, 2023

Choose a reason for hiding this comment

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

Heh no worries. I'm just having trouble transmitting the comma with pdsend, i.e. something, f32 gets interpreted as 2 messages. So perhaps a special escape sequence for the comma would suffice? So \, gets transmitted as-is, but , gets handled somehow differently (tcl->pd)

Copy link
Author

Choose a reason for hiding this comment

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

void canvas_got_clipboard_contents(t_canvas *x, t_floatarg flagf, t_symbol *s) {
    const char *debug_str = "#X msg 78 108 foo \\, bar\, f 30;";
    t_binbuf *temp_bb = binbuf_new();
    binbuf_text(temp_bb, debug_str, strlen(debug_str));
    canvas_dopaste(x, temp_bb);
    binbuf_free(temp_bb);
}

Works just fine, but I can't get it transmitted via pdsend properly

Copy link
Contributor

@umlaeute umlaeute Sep 28, 2023

Choose a reason for hiding this comment

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

that's because a plain comma in a message is a message separator in Pd lingo.
also, a line in a patch file might not be terminated by a semicolon, in which case it is continued on the next line:

#X obj 79 136 print hello;

is really equivalent to

#X
obj
79
136
print
hello;

i think this is currently not handled correctly (but haven't actually checked)

this is currently not handled correctly, and i get:

bad arguments for message 'got-clipboard-contents' to object 'canvas'
bad arguments for message 'got-clipboard-contents' to object 'canvas'
#: no such object 
ob: no such object 
prin: no such object 
hello: no such object 

iirc, miller's suggestion was:

  1. send a start-message to initiate the binbuf build (aka got-clipboard-contents 0)
  2. send the text string
  3. send an end-message to stop the binbuf build (aka got-clipboard-contents 2)
  4. after that, the text buffer is complete, and can be converted into a binbuf and copied into the patch

Copy link
Contributor

Choose a reason for hiding this comment

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

here's a super stupid (and simple idea):
do not attempt to send the data as a symbol (it's going to pollute the symbol table anyhow), but as a list of numbers (UTF-8-encoded).
This is trivial to reconstruct on the receiving side.

Copy link
Author

@eeropic eeropic Sep 28, 2023

Choose a reason for hiding this comment

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

I don't think it's stupid :-) it's like, stream of pure data... (eh) will try. Would love this to take as few lines as possible and maximize utilizing existing functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Great stuff!! Just need to implement the chunking, so it doesn't cut off the data in pdsend

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. It's just a poc.

I'm pretty sure there is some memory overallocation and what not.
On rethinking, I also would prefer "reset" and "submit" instead of "begin" and "end".

@porres
Copy link
Contributor

porres commented Apr 10, 2024

really hoping this gets done :)

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

3 participants