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

Adding support for gleam programming language #2280

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Vynlar
Copy link

@Vynlar Vynlar commented Apr 1, 2024

This pull request is definitely not ready to merge yet but I wanted to get my progress out there in case I'm going in totally the wrong direction.

I plan to add support for the gleam programming language.

Checklist

@Vynlar Vynlar changed the title Begin adding support for gleam Ading support for gleam programming language Apr 1, 2024
@Vynlar Vynlar changed the title Ading support for gleam programming language Adding support for gleam programming language Apr 1, 2024
Copy link
Author

Choose a reason for hiding this comment

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

I plan to add comments for each of these queries to explain what they're doing

Copy link
Member

Choose a reason for hiding this comment

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

Great; I'll wait to review this file in too much detail until there are more tests / comments, as that makes it easier for me to see what's going on. Lmk if there's anything in particular here that you want some feedback on in the meantime though!


[Removal] = 0:7-0:9
>--<
0| let a = 3
Copy link
Author

Choose a reason for hiding this comment

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

should this removal range include the equals?

Copy link
Member

Choose a reason for hiding this comment

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

yes that's what we usually do, esp if declaring a variable without assigning a value is supported in the language


const { supported } = ScopeSupportFacetLevel;

export const gleamScopeSupport: LanguageScopeSupportFacetMap = {
Copy link
Author

Choose a reason for hiding this comment

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

I went through the list of all of these scopes support facets and chose all the ones that I think apply to gleam. Not all of them are implemented in the queries or in the tests yet though

Copy link
Author

Choose a reason for hiding this comment

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

I also plan to tidy up this file -- right now I've just put some random stuff in for testing

Copy link
Member

Choose a reason for hiding this comment

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

Sure, tho fwiw the standard for these playground files is quite low; they're mainly just a nice-to-have. Tbh the only thing that jumps out at me here is lack of final trailing newline

Copy link
Member

@pokey pokey Apr 2, 2024

Choose a reason for hiding this comment

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

Ah nvm re trailing newline; looks like our pre-commit bot just took care of that one for you 🤖😊

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.

Great start! Left some minor comments

Comment on lines +8 to +9
[Removal] = 0:4-0:6
>--<
Copy link
Member

Choose a reason for hiding this comment

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

We usually prefer

Suggested change
[Removal] = 0:4-0:6
>--<
[Removal] = 0:0-0:9
>--------<

Comment on lines +14 to +17
[#2 Content] =
[#2 Domain] = 0:11-0:14
>---<
0| type Aaa = Int
Copy link
Member

Choose a reason for hiding this comment

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

Hmm I don't think this is usually supported in Cursorless, but I could go either way on it? cc/ @AndreasArvidsson


[Removal] = 0:7-0:9
>--<
0| let a = 3
Copy link
Member

Choose a reason for hiding this comment

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

yes that's what we usually do, esp if declaring a variable without assigning a value is supported in the language

Copy link
Member

Choose a reason for hiding this comment

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

Sure, tho fwiw the standard for these playground files is quite low; they're mainly just a nice-to-have. Tbh the only thing that jumps out at me here is lack of final trailing newline

Copy link
Member

Choose a reason for hiding this comment

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

Great; I'll wait to review this file in too much detail until there are more tests / comments, as that makes it easier for me to see what's going on. Lmk if there's anything in particular here that you want some feedback on in the meantime though!

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.

2 participants