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

Add completion for type t values #924

Merged
merged 5 commits into from
Feb 27, 2024

Conversation

DZakh
Copy link
Contributor

@DZakh DZakh commented Feb 25, 2024

An example from our project. We have a Money.zero value and now it's also included in the autocomplete:

image
// Money.res
type t = string

let zero: t = "sdf"

let make = (): t => zero

let fromInt = (int): t => int->Js.Int.toString
// Money.resi
type t

let zero: t

let make: unit => t

let fromInt: int => t

@DZakh
Copy link
Contributor Author

DZakh commented Feb 26, 2024

@zth It's ready for review

@zth
Copy link
Collaborator

zth commented Feb 26, 2024

@DZakh awesome! Have a look as soon as I can.

Copy link
Collaborator

@zth zth left a comment

Choose a reason for hiding this comment

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

This looks good to me, nice work! Was it OK to find your way around the repo/code?

There's a spelling mistake to fix, and then a changelog would be good as well, but then it's good to go.

@DZakh DZakh requested a review from zth February 27, 2024 07:05
@DZakh
Copy link
Contributor Author

DZakh commented Feb 27, 2024

This looks good to me, nice work! Was it OK to find your way around the repo/code?

Yes, it was the first time I worked on an IDE extension and it happened to be easier than I expected.

I usually start with setting up the environment, running tests, and checking that changing code actually affects something. So I followed the CONTRIBUTING.md, which felt a little bit scary, but doing things step by step until the Test section worked for me without a problem.

As for the code structure, I don't really know anything besides the code I touched. I just opened the PR #884 and found a place I need, so I have no idea about other parts of the project.

@zth
Copy link
Collaborator

zth commented Feb 27, 2024

This looks good to me, nice work! Was it OK to find your way around the repo/code?

Yes, it was the first time I worked on an IDE extension and it happened to be easier than I expected.

I usually start with setting up the environment, running tests, and checking that changing code actually affects something. So I followed the CONTRIBUTING.md, which felt a little bit scary, but doing things step by step until the Test section worked for me without a problem.

As for the code structure, I don't really know anything besides the code I touched. I just opened the PR #884 and found a place I need, so I have no idea about other parts of the project.

Ping me the next time you're looking at something in the repo and I can show you a bit more around debugging etc.

@zth zth merged commit b5fcd07 into rescript-lang:master Feb 27, 2024
5 checks passed
@DZakh
Copy link
Contributor Author

DZakh commented Feb 27, 2024

I found it quite easily myself. I like to dig down myself first, this way I have better understanding of the project.

@DZakh
Copy link
Contributor Author

DZakh commented Feb 27, 2024

The only problem, that I have a fear to start contributing sometimes 😅

@zth
Copy link
Collaborator

zth commented Feb 27, 2024

There are tools to dig easier. That's what I mean.

aspeddro pushed a commit to aspeddro/rescript-vscode that referenced this pull request Mar 4, 2024
* Add completion for type t values

* Update snapshots

* Add test

* Add test for labeled arg as well

* Fixes after review
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