-
Notifications
You must be signed in to change notification settings - Fork 645
add go.mod and go.sum support #2344
add go.mod and go.sum support #2344
Conversation
Thanks for the PR @calebdoxsey. Looks like this is your first contribution to this project, Thanks & Welcome!
Which PR are you referring to here? |
Here's the PR in the atom repo: atom/language-go#156 |
@brainsnail Do you know if there is an ETA for your PR (atom/language-go#156)? If not, then would you be interested in reviewing this PR and give us your feedback? |
@ramya-rao-a As far as I know there hasn't been an update on that PR, but I'm fine with taking a look at this PR instead 😄 I'll take a look at it this soon @calebdoxsey. |
}, | ||
"semver": { | ||
"comment": "Semver version strings (v1.2.3)", | ||
"match": "v(?:0|[1-9]\\d*)\\.(?:0|[1-9]\\d*)\\.(?:0|[1-9]\\d*)(?:-[\\da-z-]+(?:\\.[\\da-z-]+)*)?(?:\\+[\\da-z-]+(?:\\.[\\da-z-]+)*)?", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "official" regexp is: ^v[0-9]+\.(0\.0-|\d+\.\d+-([^+]*\.)?0\.)\d{14}-[A-Za-z0-9]+(\+incompatible)?$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's for the special pseudo version: https://tip.golang.org/cmd/go/#hdr-Pseudo_versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I agree about using that regex instead - I wrote one by hand on the Atom repo, but we may want to just use the same regex as the one the language uses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is it only matches pseudo versions:
v0.0.0-20161113214103-89cd22812c4f
It won't match
v1.2.3
At least that's how I read the regex. Is there another one that matches both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right - the code they use to validate the semver is here.
In testing out your branch locally I didn't experience any problems.
I tried it out with a few different repos that had modules enabled and did some Super glad to see it moving forward 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if the official regex works, then we should use that, but that's really it. Thanks so much for adding the grammars in here 🎊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @calebdoxsey 👍
Thanks a lot @brainsnail and @calebdoxsey! |
For: #1886
I was 90% done with this before I realized there was already a PR out there from a few months ago. If this isn't useful, feel free to close.