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

Visual selection utility fixes #265

Merged
merged 7 commits into from
Sep 20, 2024

Conversation

mehalter
Copy link
Contributor

@mehalter mehalter commented Sep 20, 2024

While playing around with the visual selection functionality I found a couple bugs.

  1. When using visual line mode the functionality was just broken. Mainly because when visual line mode is used, the end_col and start_col will be out of bounds of the line. This makes sure to only trim the lines if the start_col and end_col are in the bounds of the lines
  2. With the new ability to get the visual mode to pass into update_prefills I noticed that the current method for "leave visual mode" actually puts you into selection mode in the already open instance of grug-far which is very disorienting. This changes the method of leaving visual mode before getting the selection so that when updating the prefills you end up in the mode you expect in the grug-far window. While fixing this, I also normalized the use of this functionality to a utility function so there is a single place to change how to "leave visual mode"
  3. Adds a second function get_current_visual_selection_split. This is useful internally and could be useful for the end user.
  4. Refactors the code so that all locations where the visual selection is retrieved it is gotten through get_current_visual_selection_split so the logic is centralized and easier to maintain in the future and to populate bug fixes across all locations in the code.

Let me know what you think!

@mehalter mehalter requested a review from MagicDuck as a code owner September 20, 2024 07:05
@mehalter mehalter force-pushed the visual_selection_util branch 2 times, most recently from 39da4f0 to 7c5bcbf Compare September 20, 2024 07:09
@mehalter mehalter force-pushed the visual_selection_util branch from 7c5bcbf to 01a5554 Compare September 20, 2024 07:15
Copy link
Owner

@MagicDuck MagicDuck left a comment

Choose a reason for hiding this comment

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

thank you so much for the fixes! Requested changes are mostly renames 😄

doc/grug-far.txt Outdated Show resolved Hide resolved
lua/grug-far.lua Outdated Show resolved Hide resolved
lua/grug-far.lua Outdated Show resolved Hide resolved
lua/grug-far/utils.lua Show resolved Hide resolved
@mehalter
Copy link
Contributor Author

Thanks so much for the review! I applied the requested changes 😄

doc/grug-far.txt Outdated Show resolved Hide resolved
Copy link
Owner

@MagicDuck MagicDuck left a comment

Choose a reason for hiding this comment

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

Only one tiny nitpick about mentioning strict param in help doc. Otherwise all good!

@mehalter
Copy link
Contributor Author

Ah, nice catch. Just added the parameter to the docs

@mehalter mehalter requested a review from MagicDuck September 20, 2024 18:10
Copy link
Owner

@MagicDuck MagicDuck left a comment

Choose a reason for hiding this comment

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

great!

@MagicDuck MagicDuck merged commit 128e6f3 into MagicDuck:main Sep 20, 2024
5 checks passed
@mehalter mehalter deleted the visual_selection_util branch September 20, 2024 18:16
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.

2 participants