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(package): added showing gradle version based on the gradle.properties file #4432

Merged
merged 9 commits into from Oct 30, 2022

Conversation

BattleCh1cken
Copy link
Contributor

@BattleCh1cken BattleCh1cken commented Oct 2, 2022

Supplies an additional way to get gradle version.

Description

I added a condition in the get_gradle_version that checks if the version in build.gradle doesn't exist, and then gets the version from gradle.properties instead.
I'm fairly certain that the way I implemented this is very janky, but I don't know how to fix it.

Motivation and Context

Closes #4410

Screenshots (if appropriate):

How Has This Been Tested?

I ran cargo test.

I created a new test that replicated a working directory with a gradle.properties file.

  • 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.

src/modules/package.rs Outdated Show resolved Hide resolved
src/modules/package.rs Outdated Show resolved Hide resolved
src/modules/package.rs Outdated Show resolved Hide resolved
BattleCh1cken and others added 2 commits October 3, 2022 06:20
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
@davidkna
Copy link
Member

davidkna commented Oct 6, 2022

Please fix any clippy issues. I would also prefer if both files are considered in case the first doesn't have the version, but the other one does.
Try using something like this:

context.read_file_from_pwd("build.gradle")
    .and_then(|contents| {
        // ...
    }).or_else(|| {
        // try other file
    })

@BattleCh1cken
Copy link
Contributor Author

BattleCh1cken commented Oct 6, 2022

Isn't the program already doing that? If the gradle.properties file returns none, it'll look in the other file.

It looks like clippy is yelling at me for my bad code. I wish I had found out about this earlier. Clippy is nice.

@davidkna
Copy link
Member

davidkna commented Oct 6, 2022

If the gradle.properties file returns none, it'll look in the other file.

It may exist, but not contain the version

@BattleCh1cken
Copy link
Contributor Author

OOh. Sorry, I also don't really understand how the gradle build system works. Never used it. Thanks.

@davidkna
Copy link
Member

davidkna commented Oct 6, 2022

I don't know how it works in detail either, just being prudent.

@BattleCh1cken
Copy link
Contributor Author

Do you have any idea which file I should prefer? I guess it doesn't really matter since the version shouldn't exist twice.

@davidkna
Copy link
Member

davidkna commented Oct 6, 2022

I would go with build.gradle first.

@BattleCh1cken
Copy link
Contributor Author

Alright it looks clean now.

@moritzreiter
Copy link

OOh. Sorry, I also don't really understand how the gradle build system works. Never used it. Thanks.

build.gradle/build.gradle.kts is the build script and gradle.properties is a place to store "properties" which are simple key/value pairs.

The project version is either stored in the build script directly or externalized to gradle.properties and then referenced from the build script like this (if using Kotlin):

project.properties["version"].toString()

To my understanding it wouldn't make sense to have a version literal in the build script and in the properties file as this would mean you'd have to maintain it in two places.

So I would look if there's a version to be found in gradle.properties and if nothing is found take it from build.gradle/build.gradle.kts.

By the way, thanks a lot for tackling this @BattleCh1cken!

@BattleCh1cken
Copy link
Contributor Author

No problem! Thanks for being a maintainer!

@moritzreiter
Copy link

moritzreiter commented Oct 10, 2022

No problem! Thanks for being a maintainer!

I'm not. I just felt like giving my 2 cents as I had opened the issue requesting this little enhancement.

@BattleCh1cken
Copy link
Contributor Author

Welp. Thanks for helping me regardless.

Comment on lines 113 to 116
let properties_file_contents = context.read_file_from_pwd("gradle.properties")?;
let re = Regex::new(r"version=(?P<version>.*)").unwrap();
let caps = re.captures(&properties_file_contents)?;
format_version(&caps["version"], config.version_format)
Copy link
Member

Choose a reason for hiding this comment

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

Given the comments from moritzreiter, please read the properties file first.

flake.nix Outdated Show resolved Hide resolved
@andytom andytom changed the title feat: added showing gradle version based on the gradle.properties file feat(package): added showing gradle version based on the gradle.properties file Oct 30, 2022
@andytom andytom merged commit 14ee81b into starship:master Oct 30, 2022
@andytom
Copy link
Member

andytom commented Oct 30, 2022

Thank you for your contribution @BattleCh1cken and thank you for reviewing @moritzreiter and @davidkna.

Indyandie pushed a commit to Indyandie/starship that referenced this pull request Jul 26, 2023
…rties file (starship#4432)

* feat: added showing gradle version based on the gradle.properties file

* fix: wouldn't return version

* fix: forgot to remove "version=" from returned version"

* fix: ran rustfmt

* fix: test now actually tests for something

Co-authored-by: David Knaack <davidkna@users.noreply.github.com>

* fix: the regex actually makes sense now

* fix: complete refactor of control flow

* Delete flake.nix

* changed order in which files are processed

Co-authored-by: BattleCh1cken <BattleCh1cken@Larkov.de>
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
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.

extract package version from gradle.properties
4 participants