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

When you say "chuck line target1 between target2" a single empty line sticks around. #625

Closed
RonWalker22 opened this issue Mar 29, 2022 · 4 comments · Fixed by #672
Closed
Assignees
Labels
bug Something isn't working

Comments

@RonWalker22
Copy link
Contributor

RonWalker22 commented Mar 29, 2022

All lines between target1 and target2 should be removed.

In my gif example below “ace” is }.

chuck between

@RonWalker22 RonWalker22 changed the title When you say "check line target1 between target2" a single empty line sticks around. When you say "chuck line target1 between target2" a single empty line sticks around. Mar 29, 2022
@pokey pokey added the bug Something isn't working label Mar 29, 2022
@Will-Sommers
Copy link
Collaborator

Will-Sommers commented Apr 15, 2022

Heyo, taking a look at this one. @RonWalker22 — thanks for submitting the bug. The .gif is super helpful. It looks like Cursorless is pulling in the correct context here. The selection for the target within the Delete.run command has multiple lines(in my example, I'm trying to delete two lines).

targets.at(0).selection.selection.start
{_line: 7, _character: 0}
targets.at(0).selection.selection.end
{_line: 8, _character: 0}

I thought that it had something to do with the line below having no characters, but the gif below shows that that even with text, deleting 2 lines preserves the one where the cursor is currently.

output

I need to think about what a good next step would be. This seems to be a delete specific problem, ie. if I were to lines between, I'd want to select only those lines and not any newline characters. However, if I were to want to delete, I think the fix is likely to extend the select to the line below.

@AndreasArvidsson
Copy link
Member

The problem in essence with this is that the delete/chuck command depends on leading and trailing delimiter ranges. These ranges contains the new lines, commas and so on that needs to be removed together with the target. The problem with the between modifier is that it completely discards both the start and end targets and we are now left with just a raw range in between that has no understanding of the new line delimiter that should be discarded.

@Will-Sommers
Copy link
Collaborator

@AndreasArvidsson — thanks a bunch for the context. I'm looking through the code and I think the piece you're referencing is here in the compound_targets.py file?

Specifically these lines:

def is_anchor_included(range_connective: str):
    return range_connective not in ["rangeExclusive", "rangeExcludingStart"]

def is_active_included(range_connective: str):
    return range_connective not in ["rangeExclusive", "rangeExcludingEnd"]

Here's a few questions:

  • I think what I hear you saying as well is that the type of action should perhaps have some influence on how we handle a selection?
  • What do you think a prudent "first stab" at this might be?

@AndreasArvidsson
Copy link
Member

That's the talon side of it but the issue is in the vscode extension.

  • Definitely the selection you want to remove is not the same that you want to for example copy or select. We already do this in different ways in the extension
  • I think this one will solve itself when we have the new object oriented targets so I would not touch this one for now at all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants