-
Notifications
You must be signed in to change notification settings - Fork 227
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
Use the paste file in conemu for faster turnaround #446
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hI @djhaskin987
thanks for the PR — I've left some comments inline
let paste_file = slime#config#resolve("paste_file") | ||
let result = writefile(lines, paste_file, 'bS') | ||
if result != 0 | ||
echoerr "Couldn't write to slime paste file." | ||
endif | ||
return paste_file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my initial reaction was resolve
is probably too fast to benchmark (it's not in some inner loop) 🤔
but looking at the call sites, it wouldn't hurt to return paste_file
-- so I think I'm OK with that 👍
I like checking for a writefile
error, how about avoiding the result
var? (it's only used for the if
)
let paste_file = slime#config#resolve("paste_file") | |
let result = writefile(lines, paste_file, 'bS') | |
if result != 0 | |
echoerr "Couldn't write to slime paste file." | |
endif | |
return paste_file | |
let paste_file = slime#config#resolve("paste_file") | |
if writefile(lines, paste_file, 'bS') != 0 | |
echoerr "Couldn't write to slime paste file." | |
endif | |
return paste_file |
let @* = tmp | ||
" Use the selection register to send text to ConEmu using the slime paste file | ||
let paste_file = slime#common#write_paste_file(a:text) | ||
call slime#common#system("conemuc -guimacro:%s pastefile 2 %s", [a:config["HWND"], paste_file]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all in all, I'm surprised that using the pastefile would be faster than the clipboard … 🤔
The paste file has the S flag set, which tells vim not to bother calling fsync() on the file
…
The file doesn't contain anything that needs to persist across reboots.
That wasn't my understanding of fsync
— doesn't that create a race-condition if we
- write (but skip flush)
- turn around and read the file
?
another concern is that the pastefile sticks around, holding on to your last slime send — I'm not sure everybody feels comfortable with that, so I've been avoiding it when possible (only screen
was still using it, because there were no other choice 😮💨 )
@djhaskin987 any thoughts on this? |
This PR changes conemu to use the paste file instead of the windows clipboard. This can make it slightly faster if the clipboard is configured to use a subprocess like powershell.
Changes:
slime#common#write_paste_file
now returns the name of the paste file to which it wrote, so thatslime#resolve#config
doesn't have to be called twice, once in theconemu#send
and once in thewrite_paste_file
method.S
flag set, which tells vim not to bother callingfsync()
on the file, for faster write speeds. This makes sense. The file doesn't contain anything that needs to persist across reboots.This change has been "tested on my machine" and works.