Skip to content

replace treesitter #start-position with .startof#1682

Merged
pokey merged 2 commits intocursorless-dev:mainfrom
josharian:josh/start_of_end_of
Aug 2, 2023
Merged

replace treesitter #start-position with .startof#1682
pokey merged 2 commits intocursorless-dev:mainfrom
josharian:josh/start_of_end_of

Conversation

@josharian
Copy link
Collaborator

@josharian josharian commented Jul 23, 2023

This particular query pattern is going to be very common.
It's worth having syntactic sugar for.
In particular, this reduces the level of indentation
required to express that a node's associated scope
(iteration, trailing, leading, etc.)
should start or end at the start or end of a different node.

While we're here, make a related error message more helpful.

Checklist

  • I have added tests
  • [/] I have updated the docs and cheatsheet
  • [/] I have not broken the cheatsheet

@josharian josharian requested a review from pokey as a code owner July 23, 2023 22:38
@josharian
Copy link
Collaborator Author

The code of the end of the test is 😬 . Looking forward to knowing the right way to do it. And anything else. :) Be brutal.

@josharian josharian force-pushed the josh/start_of_end_of branch from 08df141 to a20f7d2 Compare July 23, 2023 23:41
@josharian
Copy link
Collaborator Author

I guess alternative approach would be to make the rewrite a method on MutableQueryCapture, and in line the forEach loop that calls it. Then the test would mostly go away. :)

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.

mostly looks fine, with some minor comments. still slightly on the fence here but seems you and @AndreasArvidsson are leaning towards it so I'll defer because I don't feel too strongly. I do wonder if we want to use startPosition rather than startOf. But again I'll defer to you and @AndreasArvidsson as I don't feel too strongly bout that either

@AndreasArvidsson
Copy link
Member

I think this is an improvement. I agree with all pokeys comments.

@pokey
Copy link
Member

pokey commented Jul 28, 2023

But @AndreasArvidsson you're leaning towards startOf over startPosition?

@AndreasArvidsson
Copy link
Member

But @AndreasArvidsson you're leaning towards startOf over startPosition?

Yes I think so

@josharian josharian force-pushed the josh/start_of_end_of branch from a20f7d2 to 2637ec3 Compare July 31, 2023 20:05
@josharian
Copy link
Collaborator Author

thanks! Please take another look.

This particular query pattern is going to be very common. 
It's worth having syntactic sugar for. 
In particular, this reduces the level of indentation 
required to express that a node's associated scope
(iteration, trailing, leading, etc.)
should start or end at the start or end of a different node.

While we're here, make a related error message more helpful.
@josharian josharian force-pushed the josh/start_of_end_of branch from 2637ec3 to 7ae1bae Compare July 31, 2023 20:21
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.

Left one tweak to make jsdoc clearer, but otherwise looks good!

@pokey pokey enabled auto-merge August 2, 2023 10:12
@pokey pokey added this pull request to the merge queue Aug 2, 2023
Merged via the queue into cursorless-dev:main with commit 6c1f724 Aug 2, 2023
@josharian josharian deleted the josh/start_of_end_of branch August 2, 2023 17:59
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