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

feat(vlang): create module #2577

Merged
merged 20 commits into from
May 3, 2021
Merged

feat(vlang): create module #2577

merged 20 commits into from
May 3, 2021

Conversation

Milo123459
Copy link
Contributor

@Milo123459 Milo123459 commented Apr 10, 2021

Description

Adds a module to display the currently installed V version

Motivation and Context

Closes #

Screenshots (if appropriate):

img

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

Copy link
Member

@vladimyr vladimyr left a comment

Choose a reason for hiding this comment

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

Following the precedent established by golang let's use vlang instead. Leaving that aside, good job @Milo123459 👍

docs/config/README.md Outdated Show resolved Hide resolved
docs/config/README.md Outdated Show resolved Hide resolved
docs/config/README.md Show resolved Hide resolved
docs/config/README.md Outdated Show resolved Hide resolved
src/configs/starship_root.rs Outdated Show resolved Hide resolved
src/modules/v.rs Outdated Show resolved Hide resolved
src/modules/v.rs Outdated Show resolved Hide resolved
src/modules/v.rs Outdated Show resolved Hide resolved
src/module.rs Outdated Show resolved Hide resolved
src/modules/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@vladimyr vladimyr left a comment

Choose a reason for hiding this comment

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

V uses v.mod files following Go's go.mod approach. @Milo123459 do a quick investigation to find out whether there are additional package management related files with v.sum being a likely candidate.

src/configs/v.rs Outdated Show resolved Hide resolved
docs/config/README.md Outdated Show resolved Hide resolved
docs/config/README.md Show resolved Hide resolved
@Milo123459
Copy link
Contributor Author

Question, does detect_extensions have to always have that, but also have all the required files?
For example, would:
hello.v without v.mod display?
Or would it need hello.v & v.mod to display?

@Milo123459
Copy link
Contributor Author

May as well add a version system for V.

@vladimyr
Copy link
Member

Question, does detect_extensions have to always have that, but also have all the required files?
For example, would:
hello.v without v.mod display?
Or would it need hello.v & v.mod to display?

If any of the conditions are met module will get displayed so it is OR rather than AND.

@vladimyr
Copy link
Member

May as well add a version system for V.

Now I lost you. What do you mean by version system?

@Milo123459
Copy link
Contributor Author

May as well add a version system for V.

Now I lost you. What do you mean by version system?

I updated the package module to display the V version if v.mod is present.

docs/config/README.md Outdated Show resolved Hide resolved
src/modules/package.rs Outdated Show resolved Hide resolved
Copy link
Member

@vladimyr vladimyr left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

src/configs/mod.rs Outdated Show resolved Hide resolved
Co-authored-by: David Knaack <[email protected]>
Copy link
Member

@vladimyr vladimyr left a comment

Choose a reason for hiding this comment

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

@Milo123459 Milo123459 changed the title feat(v): create module feat(vlang): create module Apr 20, 2021
@Milo123459
Copy link
Contributor Author

So I'm changing the vlang config to v? I don't know why but sure

@vladimyr
Copy link
Member

So I'm changing the vlang config to v? I don't know why but sure

Honestly, this is not my invention 🙃 There is already a Go module that sets precedent by having src/configs/go.rs and src/modules/golang.rs. And as much as it may be the wrong one I'll always try to keep (naming) consistency. That being said I have zero objections to a new PR that changes both golang, vlang, you name it to use more natural filenames but it has to be consistent across the board. 😉

Copy link
Member

@vladimyr vladimyr left a comment

Choose a reason for hiding this comment

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

@Milo123459 Once again thanks for your patience and hard work!

LGTM 🚀

@Milo123459
Copy link
Contributor Author

Anything else need to change?

@andytom andytom merged commit 48f913e into starship:master May 3, 2021
@andytom
Copy link
Member

andytom commented May 3, 2021

Thank you for your contribution @Milo123459 and thank you for reviewing @vladimyr and @davidkna

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.

4 participants