Skip to content

Implemented head tail modifier that works on multiple scope types #708

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

Merged
merged 23 commits into from
Jun 20, 2022
Merged

Conversation

AndreasArvidsson
Copy link
Member

@AndreasArvidsson AndreasArvidsson commented May 31, 2022

take head funk
clear tail pair

Fixes #623

Checklist

@AndreasArvidsson AndreasArvidsson marked this pull request as ready for review June 2, 2022 11:27
@AndreasArvidsson AndreasArvidsson requested a review from pokey as a code owner June 2, 2022 11:27
@AndreasArvidsson AndreasArvidsson linked an issue Jun 2, 2022 that may be closed by this pull request
@pokey pokey mentioned this pull request Jun 5, 2022
31 tasks
Base automatically changed from pokey/issue69-Support-fully-compositional-modifiers to main June 7, 2022 12:58
@pokey
Copy link
Member

pokey commented Jun 7, 2022

Performed git surgery. Original branch at https://github.com/cursorless-dev/cursorless/tree/bak/headTail; feel free to delete if this branch looks ok

Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

Looks like merge caused test failure; I'll hold off reviewing until you get a chance to take a look

Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

I'm not in love with having to modify all the targets for this one. I think it would be pretty clean to have a capture (head | tail) <modifier>? that can accept any modifier, and then constructs a new modifier that runs its input modifier and then takes the head. I think we could do that without having to touch any files

@AndreasArvidsson
Copy link
Member Author

AndreasArvidsson commented Jun 16, 2022

The problem with that approach is that we want to be able to say things like take head that or take tail air. My point is that it's not just modifiers that can be proceeded by head or tail. We even have the implicit one take head
Another option is that instead of storing it in the targets themselves the pipeline could have that information in the context maybe?

@pokey
Copy link
Member

pokey commented Jun 16, 2022

No the trailing modifier is optional, and defaults to "line". So "take head" will still be valid

@AndreasArvidsson
Copy link
Member Author

AndreasArvidsson commented Jun 16, 2022

Okay that is what you mean with the question mark. I would use square brackets for optionals in talon commands.
I guess that could work. I will give it a try

@pokey
Copy link
Member

pokey commented Jun 16, 2022

Right sorry yeah couldn't remember the syntax without a talon file in front of me so just fell back to regex 😅

@AndreasArvidsson
Copy link
Member Author

No problem I understood that as soon as I grasped your intention :)
I will give it a try!

@pokey
Copy link
Member

pokey commented Jun 16, 2022

The one thing to think about is what happens if they say something like "head every key". We may want to infer "head line every key"?

@AndreasArvidsson
Copy link
Member Author

Maybe. Maybe we should leave that one for now and try it out first?

@AndreasArvidsson
Copy link
Member Author

@pokey This implementation I think is more to your liking :)

@AndreasArvidsson
Copy link
Member Author

AndreasArvidsson commented Jun 16, 2022

By the way now we are actually free with the ordering. If we want we could have take funk head instead of take head funk. The first one is probably gone a hit dfa on public more.

@pokey pokey merged commit ac4c153 into main Jun 20, 2022
@pokey pokey deleted the headTail branch June 20, 2022 18:17
cursorless-bot pushed a commit that referenced this pull request Jun 20, 2022
* Implemented head tail modifier that works on multiple scope types

* Fixed test that broke in merge

* Cleanup

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* More cleanup

* More cleanup

* More cleanup

* Fixed line endings

* Updated tests

* Updated tests

* Head and tail modifiers swallow all following modifiers

* Updated tests

* Cleanup

* Cleanup

* Switch head tail stage to use token target instead of plane target

* Cleanup head tail stage

* Reinstate doc string

Co-authored-by: Pokey Rule <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
thetomcraig-aya pushed a commit to thetomcraig/cursorless that referenced this pull request Mar 27, 2024
…rsorless-dev#708)

* Implemented head tail modifier that works on multiple scope types

* Fixed test that broke in merge

* Cleanup

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* More cleanup

* More cleanup

* More cleanup

* Fixed line endings

* Updated tests

* Updated tests

* Head and tail modifiers swallow all following modifiers

* Updated tests

* Cleanup

* Cleanup

* Switch head tail stage to use token target instead of plane target

* Cleanup head tail stage

* Reinstate doc string

Co-authored-by: Pokey Rule <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

Implement head and tail equivalent modifier for other scopes
2 participants