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.

Sign up for GitHub

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

Jump to bottom

Auto pair deletions with selections #7269

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Auto pair deletions with selections #7269

wants to merge 8 commits into from

Conversation

dead10ck
Copy link
Member

@dead10ck dead10ck commented Jun 7, 2023 •
edited
Loading

This PR completes auto pair deletions. Currently, pairs are only deleted on ranges that are a single grapheme in width, since the normal change mapping process results in an incorrect selection on multi-grapheme ranges.

This is achieved with two new functions:

  • change_by_and_with_selection
  • delete_by_and_with_selection

They are helpers which take a transformation function that produces individual changes, one range at a time, and can optionally produce a new range that replaces the input range that it took as an argument. When one is not given, the input range is mapped through the change set as normal. This allows the caller to apply changes to the text and incrementally build a new selection as they go, picking and choosing in which cases new ranges are computed explicitly, and in which they are mapped normally.

Concretely, this solves the problem of needing auto pair deletes to work concurrently with dedenting on backspace; the auto pair hooks were changed to operate on individual ranges and return individual changes, rather than producing a whole complete transaction. Then deleting was enabled to compose changes from all 3 possible scenarios at once, i.e. deleting a single grapheme, dedenting, and auto pair deletions.

asciicast

Additionally, this PR inserts an extra whitespace when the cursor is inside a pair to pad the left and right.

asciicast

@kirawi kirawi added C-enhancement Category: Improvements A-helix-term Area: Helix term improvements labels Jun 15, 2023
@dead10ck dead10ck mentioned this pull request Jun 20, 2023
Closed
@archseer archseer added this to the next milestone Aug 1, 2023
@pascalkuthe pascalkuthe added S-waiting-on-review Status: Awaiting review from a maintainer. E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Aug 8, 2023
@dead10ck dead10ck mentioned this pull request Aug 23, 2023
Open
@the-mikedavis the-mikedavis linked an issue Aug 23, 2023 that may be closed by this pull request
Auto pairs incorrectly shifts selections with crlf line endings #8043
Open
@dead10ck dead10ck force-pushed the auto-pair-delete branch 2 times, most recently from 63992e2 to 22fa4d6 Compare April 7, 2024 19:40
@dead10ck
Copy link
Member Author

dead10ck commented Apr 7, 2024

This was rebased. I think the new tests had been failing since the integration test refactor, but I fixed those too.

@dead10ck dead10ck force-pushed the auto-pair-delete branch 2 times, most recently from 37c68cb to ff3ea3d Compare April 29, 2024 03:14
@dead10ck
Add Transaction::change_by_and_with_selection
f1b459b 
Adds `Transaction::change_by_and_with_selection` which centralizes
logic for producing change sets with a potentially new selection that
is applied incrementally, rather than all at once at the end with
`with_selection`. It also centralizes the offset tracking logic so that
the caller can construct a new selection with ranges as if they were
operating on the text as-is.
@dead10ck
backfill auto pair delete tests
c7b2d73
@dead10ck
Change auto pair hook to operate on single changes
f0d8662 
Change the auto pair hook to operate on single ranges to allow
transactions that mix auto pair changes with other operations, such as
inserting or deleting a single char, and denendting.
@dead10ck
add delete_by_and_with_selection
b1d17e7
@dead10ck
Delete pairs with multi-char-range selections
00c75e6 
This completes auto pair deletions. Currently, auto pairs only get
deleted when the range is a single grapheme wide, since otherwise,
the selection would get computed incorrectly through the normal change
mapping process. Now auto pairs get deleted even with larger ranges, and
the resulting selection is correct.
@dead10ck
insert double whitespace inside pair
8711b9e
@dead10ck
account for overlapping deletes
2456a65
@dead10ck
fix as-is tests
9360563
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements E-medium Call for participation: Experience needed to fix: Medium / intermediate S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.

Auto pairs incorrectly shifts selections with crlf line endings
4 participants
@dead10ck @archseer @pascalkuthe @kirawi