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

migrate Go func, func name, and lambda to treesitter #1756

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

josharian
Copy link
Collaborator

The previous definitions were mostly correct,
although they were missing the possibility
of declaring a function without a body.
(Understandably. They are rare.)
See the end of https://go.dev/ref/spec#Function_declarations.

Checklist

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

The previous definitions were mostly correct,
although they were missing the possibility
of declaring a function without a body.
(Understandably. They are rare.)
See the end of https://go.dev/ref/spec#Function_declarations.
@josharian josharian requested a review from pokey as a code owner August 8, 2023 22:54
queries/go.scm Outdated Show resolved Hide resolved
queries/go.scm Show resolved Hide resolved
queries/go.scm Show resolved Hide resolved
queries/go.scm Outdated Show resolved Hide resolved
queries/go.scm Show resolved Hide resolved
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 good to me! Still a couple open discussions but nothing that should stop this from merging

mark: {type: decoratedSymbol, symbolColor: default, character: b}
usePrePhraseSnapshot: true
initialState:
documentContents: _ = func() { /* body */ }
Copy link
Member

Choose a reason for hiding this comment

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

Wow lambdas really do look similar to named functions in Go. I wonder if we should just make "funk" a superset of "lambda" in Go

Copy link
Collaborator Author

@josharian josharian Aug 12, 2023

Choose a reason for hiding this comment

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

I like that. I'll do it.

I would really like to set up a per-language docs page skeleton (#1642), so I can start adding notes to it as I go. I'm a bit concerned about the perfect (autogenerated! list of all scopes! etc.) being the enemy of the good here, and forgetting useful details along the way.

I'll mark this PR as to-discuss for that purpose.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, with a simple test. If/when this merges, will update the wiki.

Copy link
Member

Choose a reason for hiding this comment

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

Wow lambdas really do look similar to named functions in Go. I wonder if we should just make "funk" a superset of "lambda" in Go

@AndreasArvidsson any objections to this one? Technically inconsistent with other languages but if you look at the syntax they're very similar to names functions

@josharian is it common to pass a short anonymous function directly as a function call arg? That's the main place where we prefer not to get trapped by "funk" in javascript

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is it common to pass a short anonymous function directly as a function call arg?

Not super common, but not rare either.

To my mind the most important aspect is that IMHO it is rare to be looking at a piece of code and not know whether it is in an anonymous func or not. It's a combination of syntax and usage. And typescript it is common for code in lambdas to outnumber the rest of the code by a significant factor, and to have that fact be relatively locally invisible.

There is long-standing discussion about adding more concise lambdas to the language, e.g. golang/go#21498. If that went in, I can definitely see the desire to use lambda for them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But I don't feel super strongly.

Copy link
Member

Choose a reason for hiding this comment

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

If it's hard to tell them apart I would be perfectly fine with supporting funk. It would be really irritating if you had to think about the syntax when using cursorless.

Copy link
Member

Choose a reason for hiding this comment

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

Cool sounds like we're on the same page then

Copy link
Member

Choose a reason for hiding this comment

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

I would really like to set up a per-language docs page skeleton (#1642), so I can start adding notes to it as I go. I'm a bit concerned about the perfect (autogenerated! list of all scopes! etc.) being the enemy of the good here, and forgetting useful details along the way.

I'll mark this PR as to-discuss for that purpose.

I'm not sure we should have this PR be to-discuss for that purpose, as that seems outside the scope of this PR. Prob better to mark that issue to-discuss and merge this PR

@josharian josharian added to discuss Plan to discuss at meet-up and removed to discuss Plan to discuss at meet-up labels Aug 12, 2023
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.

👍

@pokey pokey added this pull request to the merge queue Aug 15, 2023
Merged via the queue into cursorless-dev:main with commit f23ca12 Aug 15, 2023
@josharian josharian deleted the josh/go-func branch August 15, 2023 23:31
@pokey pokey added the lang-go Issues related to Go programming language support label Aug 24, 2023
@pokey pokey added the scope-migration Migrating scopes to next-gen scope implementation label Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-go Issues related to Go programming language support scope-migration Migrating scopes to next-gen scope implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants