You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/11/22 23:10:07 UTC
[GitHub] [guacamole-server] mike-jumper commented on a diff in pull request #402: GUACAMOLE-1586: Wrapped lines now clipped into 1.
mike-jumper commented on code in PR #402:
URL: https://github.com/apache/guacamole-server/pull/402#discussion_r1029897235
##########
src/terminal/select.c:
##########
@@ -360,16 +360,19 @@ void guac_terminal_select_end(guac_terminal* terminal) {
else {
/* Store first row */
- guac_terminal_clipboard_append_row(terminal, start_row, start_col, -1);
+ guac_terminal_buffer_row* buffer_row = guac_terminal_buffer_get_row(terminal->buffer, start_row, 0);
+ guac_terminal_clipboard_append_row(terminal, start_row, start_col, buffer_row->length - 2);
/* Store all middle rows */
for (int row = start_row + 1; row < end_row; row++) {
- guac_common_clipboard_append(terminal->clipboard, "\n", 1);
- guac_terminal_clipboard_append_row(terminal, row, 0, -1);
+ guac_terminal_buffer_row* buffer_row = guac_terminal_buffer_get_row(terminal->buffer, start_row, 0);
+ guac_terminal_clipboard_append_row(terminal, row, 0, buffer_row->length - 2);
+ if(!(buffer_row->characters[buffer_row->length - 1].value == 0 && buffer_row->characters[buffer_row->length - 2].value > 0))
Review Comment:
To match the style of established code, please include a space between `if` and the opening paren.
##########
src/terminal/select.c:
##########
@@ -360,16 +360,19 @@ void guac_terminal_select_end(guac_terminal* terminal) {
else {
/* Store first row */
- guac_terminal_clipboard_append_row(terminal, start_row, start_col, -1);
+ guac_terminal_buffer_row* buffer_row = guac_terminal_buffer_get_row(terminal->buffer, start_row, 0);
+ guac_terminal_clipboard_append_row(terminal, start_row, start_col, buffer_row->length - 2);
/* Store all middle rows */
for (int row = start_row + 1; row < end_row; row++) {
- guac_common_clipboard_append(terminal->clipboard, "\n", 1);
- guac_terminal_clipboard_append_row(terminal, row, 0, -1);
+ guac_terminal_buffer_row* buffer_row = guac_terminal_buffer_get_row(terminal->buffer, start_row, 0);
Review Comment:
Won't this repeatedly retrieve the same row?
##########
src/terminal/select.c:
##########
@@ -360,16 +360,19 @@ void guac_terminal_select_end(guac_terminal* terminal) {
else {
/* Store first row */
- guac_terminal_clipboard_append_row(terminal, start_row, start_col, -1);
+ guac_terminal_buffer_row* buffer_row = guac_terminal_buffer_get_row(terminal->buffer, start_row, 0);
+ guac_terminal_clipboard_append_row(terminal, start_row, start_col, buffer_row->length - 2);
Review Comment:
1. `buffer_row->length` is not guaranteed to be at least 2 here unless the call to `guac_terminal_buffer_get_row()` requests this.
2. Why are we universally skipping the last two characters?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org