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-1943: Add ctrl+arrows/ctrl+backspace shortcuts #505

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

corentin-soriano
Copy link
Contributor

@corentin-soriano corentin-soriano commented Apr 15, 2024

Adds support for the following hotkeys:

  • CTRL+Left arrow = move one word to the left.
  • CTRL+Right arrow = move one word to the right.
  • CTRL+Backspace = delete the word to the left (allow to avoid closing the tab outside of full screen mode with CTRL+w).
  • CTRL+Suppr = the opposite
  • Shift+Up/Down arrow = same as shift + page up/down but one line

@corentin-soriano corentin-soriano force-pushed the GUACAMOLE-1943_ctrl_keyboard_shortcuts_add branch from caab28b to 7250202 Compare May 30, 2024 09:39
@corentin-soriano corentin-soriano force-pushed the GUACAMOLE-1943_ctrl_keyboard_shortcuts_add branch from 7250202 to 4648128 Compare June 6, 2024 17:04
Copy link
Contributor

@necouchman necouchman left a comment

Choose a reason for hiding this comment

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

Just one request - #define things that should be constants.

@corentin-soriano
Copy link
Contributor Author

Just one request - #define things that should be constants.

Shouldn’t I rather follow the existing style where the key/terminal codes are currently written without #define?

@necouchman
Copy link
Contributor

Just one request - #define things that should be constants.

Shouldn’t I rather follow the existing style where the key/terminal codes are currently written without #define?

Well, we're trying to improve that:

https://issues.apache.org/jira/browse/GUACAMOLE-287

@corentin-soriano corentin-soriano force-pushed the GUACAMOLE-1943_ctrl_keyboard_shortcuts_add branch 2 times, most recently from 55c3b2a to e2785f8 Compare June 17, 2024 10:15
@corentin-soriano
Copy link
Contributor Author

Just one request - #define things that should be constants.

Shouldn’t I rather follow the existing style where the key/terminal codes are currently written without #define?

Well, we're trying to improve that:

https://issues.apache.org/jira/browse/GUACAMOLE-287

While I was at it, I added #define constants for the entirety of the __guac_terminal_send_key function and broke up overly long lines.

Feel free to let me know if I should move the declarations to another file or if it would be better to comment each #define.

Would you prefer that I move the second commit to a new pull request?"

@corentin-soriano corentin-soriano force-pushed the GUACAMOLE-1943_ctrl_keyboard_shortcuts_add branch from e2785f8 to 47e04b8 Compare June 17, 2024 14:16
@necouchman
Copy link
Contributor

Just one request - #define things that should be constants.

Shouldn’t I rather follow the existing style where the key/terminal codes are currently written without #define?

Well, we're trying to improve that:
https://issues.apache.org/jira/browse/GUACAMOLE-287

While I was at it, I added #define constants for the entirety of the __guac_terminal_send_key function and broke up overly long lines.

Feel free to let me know if I should move the declarations to another file or if it would be better to comment each #define.

Would you prefer that I move the second commit to a new pull request?"

Wow, thank you. I'm good with them all being in this pull request, but could you move them to a header file - guessing terminal.h?

As far as comments go, to be consistent we'll probably need a comment/documentation for each #define, but I'll let @mike-jumper or @jmuehlner add any comments they have on that...

@corentin-soriano corentin-soriano force-pushed the GUACAMOLE-1943_ctrl_keyboard_shortcuts_add branch from 47e04b8 to 0e9c3e5 Compare June 18, 2024 06:45
@corentin-soriano
Copy link
Contributor Author

Just one request - #define things that should be constants.

Shouldn’t I rather follow the existing style where the key/terminal codes are currently written without #define?

Well, we're trying to improve that:
https://issues.apache.org/jira/browse/GUACAMOLE-287

While I was at it, I added #define constants for the entirety of the __guac_terminal_send_key function and broke up overly long lines.
Feel free to let me know if I should move the declarations to another file or if it would be better to comment each #define.
Would you prefer that I move the second commit to a new pull request?"

Wow, thank you. I'm good with them all being in this pull request, but could you move them to a header file - guessing terminal.h?

As far as comments go, to be consistent we'll probably need a comment/documentation for each #define, but I'll let @mike-jumper or @jmuehlner add any comments they have on that...

#defines moved to terminal.h
If necessary, I will add a comment by #define rather than by block.

@corentin-soriano
Copy link
Contributor Author

@jmuehlner, @mike-jumper, would you prefer that I add a comment for each #define or should I leave it like this?

@jmuehlner
Copy link
Contributor

@jmuehlner, @mike-jumper, would you prefer that I add a comment for each #define or should I leave it like this?

To match our guidelines and existing style, each constant (whether defined by #define or otherwise) should have a seperate comment.

@corentin-soriano corentin-soriano force-pushed the GUACAMOLE-1943_ctrl_keyboard_shortcuts_add branch 2 times, most recently from 78f01a8 to a70f766 Compare August 27, 2024 08:51
@corentin-soriano
Copy link
Contributor Author

@jmuehlner, @mike-jumper, would you prefer that I add a comment for each #define or should I leave it like this?

To match our guidelines and existing style, each constant (whether defined by #define or otherwise) should have a seperate comment.

I added a comment for each #define and performed a rebase to update the branch.

@corentin-soriano corentin-soriano force-pushed the GUACAMOLE-1943_ctrl_keyboard_shortcuts_add branch from a70f766 to 0813e93 Compare October 6, 2024 17:06
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