GUACAMOLE-1973: Add support for XTerm bracketed-paste mode#533
GUACAMOLE-1973: Add support for XTerm bracketed-paste mode#533scottp-dpaw wants to merge 1 commit intoapache:mainfrom
Conversation
6c345ca to
1a21a56
Compare
|
@scottp-dpaw Since this is a new feature, and not a bug fix, this can be based against the |
necouchman
left a comment
There was a problem hiding this comment.
I have two overall concerns with the code, here, as illustrated by the one comment I've left inline, below:
- You've done a reasonable job of documenting things within the code, but there are a few places that could use slightly more documentation.
- There are several places where constants may be useful.
Other than that, I'll let @mike-jumper and @jmuehlner take a closer look, since they're much more familiar with the terminal code than I am.
src/terminal/terminal.c
Outdated
| } | ||
|
|
||
| int guac_terminal_send_clipboard(guac_terminal *term) { | ||
| char *filtered = guac_mem_alloc(term->clipboard->length + 12); |
There was a problem hiding this comment.
It may be completely obvious to you why you need to allocate 12 bytes in addition to the clipboard length, here, but it isn't to me. This is one example of a place where 1) more comments throughout the code might be helpful, and 2) this may be a good place for a constant to replace the literal 12.
1a21a56 to
d0fc2f8
Compare
|
No worries. In lieu of making constants for every bitmask, I've added clearer comments describing the sequence of bits that we're trying to find for each codepoint. Hopefully that should provide enough context to the reader about what we're trying to do, if not I'm open to suggestions. |
d0fc2f8 to
6021ddc
Compare
6021ddc to
25206c9
Compare
corentin-soriano
left a comment
There was a problem hiding this comment.
This seems to work.
I agree with Nick, it would be nice to have constants on some complex values.
| /* Exclude Unicode CO (U+0000 to U+001F) control characters, except | ||
| * for tab (U+0009), line feed (U+000A) and carriage return (U+000D). */ | ||
| if (!((src_ptr[0] >= 0x00) && (src_ptr[0] < 0x20)) || |
There was a problem hiding this comment.
Although the result is the same, I think src_ptr[0] <= 0x1f would be closer than src_ptr[0] < 0x20 to what is documented above.
No description provided.