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-192: Select word in terminal on double click #504

Merged
merged 1 commit into from
May 29, 2024

Conversation

corentin-soriano
Copy link
Contributor

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

Allows selecting a word on double-click and a complete line on triple-click.

Based on the unmaintained PR by @Saiyeux (#406) :

  • All "rewritten" functions have been removed in favor of existing functions.
    -> Code simplification.
    -> Fixes several unexpected behaviors when scrolling upwards.
  • The following characters are now considered part of a "word": [0-9\/A-Za-z_\~\&\-\.\?\\%=:$]
    -> Allows selecting an IP address, a path or URL (ex: https://www.test.tld/file.php?param1=value1&param2=value%202)
  • Improved condition in the guac_terminal_select_update (select.c) function to allow selecting a "word" of a single character on double-click (no impact on single-click).

@corentin-soriano corentin-soriano marked this pull request as draft April 15, 2024 13:53
@corentin-soriano corentin-soriano marked this pull request as ready for review April 15, 2024 14:23
src/terminal/terminal.c Outdated Show resolved Hide resolved
@jmuehlner
Copy link
Contributor

This looks good to me, aside from a minor punctuation mistake. My only other request would be that some of these commits be reworded and / or squashed.

Each commit should:

  • Follow the format GUACAMOLE-1234: Clarify commit message expectations., with the first word capitalized, and a period after the message.
  • Cleary explain what the commit is intended to do.

Something like GUACAMOLE-192: revert removed line is unclear about what line is being reverted, and whether it's a substantive change or not. If it's just a minor whitespace fix, I'd suggest just squashing it into whatever commit its related to.

@corentin-soriano
Copy link
Contributor Author

This looks good to me, aside from a minor punctuation mistake. My only other request would be that some of these commits be reworded and / or squashed.

Each commit should:

  • Follow the format GUACAMOLE-1234: Clarify commit message expectations., with the first word capitalized, and a period after the message.
  • Cleary explain what the commit is intended to do.

Something like GUACAMOLE-192: revert removed line is unclear about what line is being reverted, and whether it's a substantive change or not. If it's just a minor whitespace fix, I'd suggest just squashing it into whatever commit its related to.

Thank you for your review!

The punctuation mistake is corrected.
It was in fact not really useful to keep all of these commits which were only corrections of the first one. I squashed them.

@jmuehlner jmuehlner merged commit 7d004ce into apache:main May 29, 2024
1 check passed
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