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

GUACAMOLE-1632: Have spaces written in the appended clipboard. #404

Closed
wants to merge 4 commits into from

Conversation

Saiyeux
Copy link

@Saiyeux Saiyeux commented Nov 22, 2022

Add end of line identification when writing to the clipboard, avoiding converted spaces.

Add end of line identification when writing to the clipboard, avoiding converted spaces.
Add end of line identification when writing to the clipboard, avoiding converted spaces.
Copy link
Contributor

@mike-jumper mike-jumper left a comment

Choose a reason for hiding this comment

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

Please also take a look at your commit messages:

Commit messages from PR #404

Commits should cover logical changes and describe the nature of those changes at a high level. You shouldn't end up with multiple commits having exactly the same message.

Comment on lines +319 to +321
/* Keep consistency with vim display */
if (codepoint == 0)
codepoint = 32;
Copy link
Contributor

Choose a reason for hiding this comment

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

Certainly this isn't a workaround for some vim-specific behavior, but rather a missing aspect of the correct way for a terminal emulator to behave? Can you rephrase to capture the nature of what's being done here?

Comment on lines 300 to 305
int j = end;
while((!buffer_row->characters[j].value)&& (j > start)){
j--;
}
if(j != start)
end = j;
Copy link
Contributor

Choose a reason for hiding this comment

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

Without any comments/documentation here, it's pretty unclear what the intent or purpose of this block of code is. Can you clarify?

Comment on lines 301 to 304
while((!buffer_row->characters[j].value)&& (j > start)){
j--;
}
if(j != start)
Copy link
Contributor

Choose a reason for hiding this comment

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

To match the style of established code, please:

  • Include a space between while / if and the opening paren.
  • Include spaces around binary operators like &&.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I'll mark some annotations on it and re-format. After investigating we assume that this problem is due to both vim specific behavior and guaca's display & copy logic.

The function of annotated blocks is to ignore any spaces or null chars after a line ends when copying multiple lines after selection. This simple logic gives extra spaces if user select what identified as null in simulator. Thus keeps the clipboard consistent with what user sees in simulator.
@necouchman
Copy link
Contributor

Closed by #523.

@necouchman necouchman closed this Jun 11, 2024
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.

3 participants