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.

Sign up for GitHub

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

refactor: propagate version formatting errors #2566

Merged
merged 7 commits into from
Apr 10, 2021
Merged

refactor: propagate version formatting errors #2566

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b01679e
refactor: propagate version formatting errors
vladimyr Apr 8, 2021
d149669
refactor: trim version formatting boilerplate
vladimyr Apr 9, 2021
6f38d08
refactor(node): unwrap version formatting
vladimyr Apr 9, 2021
03c1a9b
docs: fix typo
vladimyr Apr 9, 2021
3638556
docs: remove dots after `version_format`
vladimyr Apr 9, 2021
bcb37d4
feat: lazy version parsing
vladimyr Apr 9, 2021
6326be6
refactor(version-formatter): collect segments into string
vladimyr Apr 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/formatter/string_formatter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type VariableMapType<'a> =
type StyleVariableMapType<'a> =
BTreeMap<String, Option<Result<Cow<'a, str>, StringFormatterError>>>;

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
pub enum StringFormatterError {
Custom(String),
Parse(PestError<Rule>),
Expand Down
39 changes: 19 additions & 20 deletions src/formatter/version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl<'a> VersionFormatter<'a> {
/// Formats a version structure into a readable string
///
/// No matter what comes in, this will return some usable string
pub fn format_version(self, version: &str) -> String {
pub fn format_version(self, version: &str) -> Result<String, StringFormatterError> {
let parsed = Versioning::new(version);
let formatted = self
.formatter
Expand All @@ -44,14 +44,14 @@ impl<'a> VersionFormatter<'a> {
_ => None,
})
.parse(None);
match formatted {
Ok(segments) => segments

formatted.map(|segments| {
segments
.iter()
.map(|segment| segment.value.as_str())
.collect::<Vec<&str>>()
.join(""),
Err(_) => version.to_string(),
}
.join("")
vladimyr marked this conversation as resolved.
Show resolved Hide resolved
})
}
}

Expand All @@ -63,35 +63,34 @@ mod tests {
fn test_semver_full() {
const FORMAT_STR: &str = "major:${major} minor:${minor} patch:${patch} raw:${raw}";
let result = VersionFormatter::new(FORMAT_STR)
.unwrap()
.format_version("1.2.3");
assert_eq!(result, "major:1 minor:2 patch:3 raw:1.2.3");
.and_then(|formatter| formatter.format_version("1.2.3"));
assert_eq!(result, Ok("major:1 minor:2 patch:3 raw:1.2.3".to_string()));
}

#[test]
fn test_semver_partial() {
const FORMAT_STR: &str = "major:${major} minor:${minor} patch:${patch} raw:${raw}";
let result = VersionFormatter::new(FORMAT_STR)
.unwrap()
.format_version("1.2");
assert_eq!(result, "major:1 minor:2 patch: raw:1.2");
let result =
VersionFormatter::new(FORMAT_STR).and_then(|formatter| formatter.format_version("1.2"));
assert_eq!(result, Ok("major:1 minor:2 patch: raw:1.2".to_string()));
}

#[test]
fn test_general() {
const FORMAT_STR: &str = "major:${major} minor:${minor} patch:${patch} raw:${raw}";
let result = VersionFormatter::new(FORMAT_STR)
.unwrap()
.format_version("1.2-a.3");
assert_eq!(result, "major:1 minor:2 patch: raw:1.2-a.3");
.and_then(|formatter| formatter.format_version("1.2-a.3"));
assert_eq!(result, Ok("major:1 minor:2 patch: raw:1.2-a.3".to_string()));
}

#[test]
fn test_mess() {
fn test_dummy() {
const FORMAT_STR: &str = "major:${major} minor:${minor} patch:${patch} raw:${raw}";
let result = VersionFormatter::new(FORMAT_STR)
.unwrap()
.format_version("utter junk");
assert_eq!(result, "major: minor: patch: raw:utter junk");
.and_then(|formatter| formatter.format_version("dummy version"));
assert_eq!(
result,
Ok("major: minor: patch: raw:dummy version".to_string())
);
}
}
70 changes: 36 additions & 34 deletions src/modules/java.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
_ => None,
})
.map(|variable| match variable {
"version" => {
let java_version = get_java_version(context, &config)?;
Some(Ok(java_version))
}
"version" => get_java_version(context, &config).map(Ok),
_ => None,
})
.parse(None)
Expand Down Expand Up @@ -74,19 +71,24 @@ fn get_java_version(context: &Context, config: &JavaConfig) -> Option<String> {
output.stdout
};

parse_java_version(config.version_format, &java_version)
format_java_version(&java_version, config.version_format)
}

fn parse_java_version(version_format: &str, java_version: &str) -> Option<String> {
fn format_java_version(java_version: &str, version_format: &str) -> Option<String> {
let re = Regex::new(JAVA_VERSION_PATTERN).ok()?;
let captures = re.captures(java_version)?;
let version = &captures["version"];

Some(
VersionFormatter::new(version_format)
.ok()?
.format_version(version),
)
let formatted = VersionFormatter::new(version_format)
.and_then(|formatter| formatter.format_version(version));

match formatted {
Ok(formatted) => Some(formatted),
Err(error) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

What conditions would lead to this warning? I would rather just silently format the raw version than log a warning when this happens.

Copy link
Member Author

@vladimyr vladimyr Apr 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I would rather just silently format the raw version than log a warning when this happens.

This is silent unless you explicitly opt-in by setting STARSHIP_LOG environment variable to the warn level.

EDIT: warn is default log level so it gets print always but that is actually a good thing if you ask me

What conditions would lead to this warning?

The simplest case that would cause an error is a malformed version format like:

version = "v${raw" # missing right curly

This results in (with logging enabled):

[WARN] - (starship::modules::java): Error formating `java` version:
 --> 1:3
  |
1 | v${raw
  |   ^---
  |
  = expected variable_name

log::warn!("Error formating `java` version:\n{}", error);
Some(format!("v{}", version))
}
}
}

#[cfg(test)]
Expand All @@ -98,106 +100,106 @@ mod tests {
use std::io;

#[test]
fn test_parse_java_version_openjdk() {
fn test_format_java_version_openjdk() {
let java_8 = "OpenJDK 64-Bit Server VM (25.222-b10) for linux-amd64 JRE (1.8.0_222-b10), built on Jul 11 2019 10:18:43 by \"openjdk\" with gcc 4.4.7 20120313 (Red Hat 4.4.7-23)";
let java_11 = "OpenJDK 64-Bit Server VM (11.0.4+11-post-Ubuntu-1ubuntu219.04) for linux-amd64 JRE (11.0.4+11-post-Ubuntu-1ubuntu219.04), built on Jul 18 2019 18:21:46 by \"build\" with gcc 8.3.0";
assert_eq!(
parse_java_version("v${raw}", java_11),
format_java_version(java_11, "v${raw}"),
Some("v11.0.4".to_string())
);
assert_eq!(
parse_java_version("v${raw}", java_8),
format_java_version(java_8, "v${raw}"),
Some("v1.8.0".to_string())
);
}

#[test]
fn test_parse_java_version_oracle() {
fn test_format_java_version_oracle() {
let java_8 = "Java HotSpot(TM) Client VM (25.65-b01) for linux-arm-vfp-hflt JRE (1.8.0_65-b17), built on Oct 6 2015 16:19:04 by \"java_re\" with gcc 4.7.2 20120910 (prerelease)";
assert_eq!(
parse_java_version("v${raw}", java_8),
format_java_version(java_8, "v${raw}"),
Some("v1.8.0".to_string())
);
}

#[test]
fn test_parse_java_version_redhat() {
fn test_format_java_version_redhat() {
let java_8 = "OpenJDK 64-Bit Server VM (25.222-b10) for linux-amd64 JRE (1.8.0_222-b10), built on Jul 11 2019 20:48:53 by \"root\" with gcc 7.3.1 20180303 (Red Hat 7.3.1-5)";
let java_12 = "OpenJDK 64-Bit Server VM (12.0.2+10) for linux-amd64 JRE (12.0.2+10), built on Jul 18 2019 14:41:47 by \"jenkins\" with gcc 7.3.1 20180303 (Red Hat 7.3.1-5)";
assert_eq!(
parse_java_version("v${raw}", java_8),
format_java_version(java_8, "v${raw}"),
Some("v1.8.0".to_string())
);
assert_eq!(
parse_java_version("v${raw}", java_12),
format_java_version(java_12, "v${raw}"),
Some("v12.0.2".to_string())
);
}

#[test]
fn test_parse_java_version_zulu() {
fn test_format_java_version_zulu() {
let java_8 = "OpenJDK 64-Bit Server VM (25.222-b10) for linux-amd64 JRE (Zulu 8.40.0.25-CA-linux64) (1.8.0_222-b10), built on Jul 11 2019 11:36:39 by \"zulu_re\" with gcc 4.4.7 20120313 (Red Hat 4.4.7-3)";
let java_11 = "OpenJDK 64-Bit Server VM (11.0.4+11-LTS) for linux-amd64 JRE (Zulu11.33+15-CA) (11.0.4+11-LTS), built on Jul 11 2019 21:37:17 by \"zulu_re\" with gcc 4.9.2 20150212 (Red Hat 4.9.2-6)";
assert_eq!(
parse_java_version("v${raw}", java_8),
format_java_version(java_8, "v${raw}"),
Some("v1.8.0".to_string())
);
assert_eq!(
parse_java_version("v${raw}", java_11),
format_java_version(java_11, "v${raw}"),
Some("v11.0.4".to_string())
);
}

#[test]
fn test_parse_java_version_eclipse_openj9() {
fn test_format_java_version_eclipse_openj9() {
let java_8 = "Eclipse OpenJ9 OpenJDK 64-bit Server VM (1.8.0_222-b10) from linux-amd64 JRE with Extensions for OpenJDK for Eclipse OpenJ9 8.0.222.0, built on Jul 17 2019 21:29:18 by jenkins with g++ (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)";
let java_11 = "Eclipse OpenJ9 OpenJDK 64-bit Server VM (11.0.4+11) from linux-amd64 JRE with Extensions for OpenJDK for Eclipse OpenJ9 11.0.4.0, built on Jul 17 2019 21:51:37 by jenkins with g++ (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)";
assert_eq!(
parse_java_version("v${raw}", java_8),
format_java_version(java_8, "v${raw}"),
Some("v1.8.0".to_string())
);
assert_eq!(
parse_java_version("v${raw}", java_11),
format_java_version(java_11, "v${raw}"),
Some("v11.0.4".to_string())
);
}

#[test]
fn test_parse_java_version_graalvm() {
fn test_format_java_version_graalvm() {
let java_8 = "OpenJDK 64-Bit GraalVM CE 19.2.0.1 (25.222-b08-jvmci-19.2-b02) for linux-amd64 JRE (8u222), built on Jul 19 2019 17:37:13 by \"buildslave\" with gcc 7.3.0";
assert_eq!(
parse_java_version("v${raw}", java_8),
format_java_version(java_8, "v${raw}"),
Some("v8".to_string())
);
}

#[test]
fn test_parse_java_version_amazon_corretto() {
fn test_format_java_version_amazon_corretto() {
let java_8 = "OpenJDK 64-Bit Server VM (25.222-b10) for linux-amd64 JRE (1.8.0_222-b10), built on Jul 11 2019 20:48:53 by \"root\" with gcc 7.3.1 20180303 (Red Hat 7.3.1-5)";
let java_11 = "OpenJDK 64-Bit Server VM (11.0.4+11-LTS) for linux-amd64 JRE (11.0.4+11-LTS), built on Jul 11 2019 20:06:11 by \"\" with gcc 7.3.1 20180303 (Red Hat 7.3.1-5)";
assert_eq!(
parse_java_version("v${raw}", java_8),
format_java_version(java_8, "v${raw}"),
Some("v1.8.0".to_string())
);
assert_eq!(
parse_java_version("v${raw}", java_11),
format_java_version(java_11, "v${raw}"),
Some("v11.0.4".to_string())
);
}

#[test]
fn test_parse_java_version_sapmachine() {
fn test_format_java_version_sapmachine() {
let java_11 = "OpenJDK 64-Bit Server VM (11.0.4+11-LTS-sapmachine) for linux-amd64 JRE (11.0.4+11-LTS-sapmachine), built on Jul 17 2019 08:58:43 by \"\" with gcc 7.3.0";
assert_eq!(
parse_java_version("v${raw}", java_11),
format_java_version(java_11, "v${raw}"),
Some("v11.0.4".to_string())
);
}

#[test]
fn test_parse_java_version_unknown() {
fn test_format_java_version_unknown() {
let unknown_jre = "Unknown JRE";
assert_eq!(parse_java_version("v${raw}", unknown_jre), None);
assert_eq!(format_java_version(unknown_jre, "v${raw}"), None);
}

#[test]
Expand Down
27 changes: 17 additions & 10 deletions src/modules/nodejs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,8 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
})
.map(|variable| match variable {
"version" => {
let nodejs_detected_version = &nodejs_version
.deref()
.as_ref()
.map(|version| version.trim());
match nodejs_detected_version {
Some(version) => Some(Ok(VersionFormatter::new(config.version_format)
.ok()?
.format_version(&version.to_string().drain(1..).collect::<String>()))),
None => None,
}
format_node_version(nodejs_version.deref().as_ref()?, config.version_format)
.map(Ok)
}
_ => None,
})
Expand Down Expand Up @@ -114,6 +106,21 @@ fn check_engines_version(nodejs_version: &str, engines_version: Option<String>)
r.matches(&v)
}

fn format_node_version(node_version: &str, version_format: &str) -> Option<String> {
let version = node_version.trim_start_matches('v').trim();

let formatted = VersionFormatter::new(version_format)
.and_then(|formatter| formatter.format_version(version));

match formatted {
Ok(formatted) => Some(formatted),
Err(error) => {
log::warn!("Error formating `node` version:\n{}", error);
Some(format!("v{}", version))
}
}
}

#[cfg(test)]
mod tests {
use crate::test::ModuleRenderer;
Expand Down
Loading