(#6615) Fixed --ls error messages by refactoring get_metadata_section #12758
Conversation
| @@ -404,19 +404,24 @@ fn get_metadata_section_imp(os: Os, filename: &Path) -> Option<MetadataBlob> { | |||
| Some(ar) => ar, | |||
| None => { | |||
| debug!("llvm didn't like `{}`", filename.display()); | |||
| return None; | |||
| return Err(format!("llvm didn't like '{}'", filename.display())); | |||
There was a problem hiding this comment.
as a user-facing error message, "llvm didn't like" is probably not the best wording, perhaps "failed to read rlib metadata: ..."
|
Alright Alex, I updated those error messages and made them lower case. |
|
Could you please reference #6615 and add Closes in the commit message? Also, could you please squash those commits into 1? |
| @@ -395,7 +395,7 @@ fn get_metadata_section(os: Os, filename: &Path) -> Option<MetadataBlob> { | |||
| return ret; | |||
| } | |||
|
|
|||
| fn get_metadata_section_imp(os: Os, filename: &Path) -> Option<MetadataBlob> { | |||
| fn get_metadata_section_imp(os: Os, filename: &Path) -> Result<MetadataBlob, ~str> { | |||
| if filename.filename_str().unwrap().ends_with(".rlib") { | |||
There was a problem hiding this comment.
You could also verify if the file exists before this if:
if !filename.exists() {
return Err(format!("No such file {}", filename.display()));
}
|
Alright FlaPer87, I squashed the commits and added your file existence check to get_metadata_section_imp. |
|
The error messages returned from this method are still a bit inconsistent. I've seen these formats: This should be consistent in the formatting of the error messages: Also, remember that all error messages start with lower case letters, there's still one that starts with a capital letter. |
|
Alright, I fixed the error messages, and the newest one requested by flapper87 which had the capital case. |
…or messages. Closes #6615.
Refactored get_metadata_section to return a Result<MetadataBlob,~str> instead of a Option<MetadataBlob>. This provides more clarity to the user through the debug output when using --ls. This is kind of a continuation of my original closed pull request 2 months ago (#11544), but I think the time-span constitutes a new pull request.
Refactored get_metadata_section to return a Result<MetadataBlob,~str> instead of a Option. This provides more clarity to the user through the debug output when using --ls.
This is kind of a continuation of my original closed pull request 2 months ago (#11544), but I think the time-span constitutes a new pull request.