-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 #926 #1254
Fix #926 #1254
Conversation
sway/commands/clipboard.c
Outdated
#include "stringop.h" | ||
|
||
static void send_clipboard(void *data, const char *type, int fd) { | ||
if (strcmp(type, "text/plain") != 0 && strcmp(type, "text/plain;charset=utf-8") != 0) { |
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.
Limit this to 80 columns please
This looks right, but I don't want to close #926 until we have the ability to read from the clipboard, too. |
sway/commands/clipboard.c
Outdated
|
||
static void send_clipboard(void *data, const char *type, int fd) { | ||
if ((strcmp(type, "text/plain") != 0) && | ||
(strcmp(type, "text/plain;charset=utf-8") != 0)) { |
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.
Sorry, more style nits. You don't need these extra parens and you should put &&
on the next line.
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.
Oversaw this, is it ok now? Still getting used to this codestyle
} | ||
|
||
const char *str = data; | ||
write(fd, str, strlen(str)); |
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.
I presume that you don't need to write the null delimiter?
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.
ugh... actually not sure it writes to the fd of a client, is there a null delimiter needed/expected?
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.
it works without at least for all common applications... the clients can determine the length from the number of bytes read
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.
It could work both ways and I'd err on the side of no null delimiter, but so long as you've tested it this way then I trust that it works.
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.
Yeah, I figured as much.
sway/commands/clipboard.c
Outdated
const char *str = join_args(argv, argc); | ||
wlc_set_selection((void*) str, types, 2, &send_clipboard); | ||
|
||
free((void*) current_data); |
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.
Casting to void*
here is unneccessary and probably on wlc_set_selection
as well.
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.
did it because str is a const pointer and wlc_set_selection/free require a non-const pointer
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.
Well why did you make it const then?
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.
oh, good point...
Looks good, thanks! Think you could squash the commits for me? |
Adds the 'clipboard' command that can currently only be used
like this: 'swaymsg clipboard Some clipboard text here'. In future it could be extended to set non-text data or read from the clipboard but this would require extensions to wlc (or has to wait for wlroots).
Note that due to sway command arguments passing this will
not preserve whitespace in the clipboard text correctly (i.e. two spaces will be interpreted as one).
Requires Cloudef/wlc#272 to work correctly.
Still not too familiar with the sway codebase, I am not sure this is the correct way to add new commands to sway, or if they have to be added somewhere else.