For brief (--short) and verbose version output, reverse the default order. Also add integration tests for both.

Author: tkuestner

src/output.rs

@@ -27,8 +27,12 @@ pub fn show_brief_version_list(
     sort_recent_first: bool,
     w: &mut dyn Write
 ) -> Result<()> {
-    for info in get_band_infos(archive, sort_recent_first)? {
-        writeln!(w, "{}", info.id)?
+    let mut band_ids = archive.list_band_ids()?;
+    if sort_recent_first {
+        band_ids.reverse();
+    }
+    for band_id in band_ids {
+        writeln!(w, "{}", band_id)?
     }
     Ok(())
 }
@@ -39,8 +43,25 @@ pub fn show_verbose_version_list(
     show_sizes: bool,
     w: &mut dyn Write,
 ) -> Result<()> {
-    for info in get_band_infos(archive, sort_recent_first)? {
-        let band_id = &info.id;
+    let mut band_ids = archive.list_band_ids()?;
+    if sort_recent_first {
+        band_ids.reverse();
+    }
+    for band_id in band_ids {
+        let band = match Band::open(&archive, &band_id) {
+            Ok(band) => band,
+            Err(e) => {
+                ui::problem(&format!("Failed to open band {:?}: {:?}", band_id, e));
+                continue;
+            }
+        };
+        let info = match band.get_info() {
+            Ok(info) => info,
+            Err(e) => {
+                ui::problem(&format!("Failed to read band tail {:?}: {:?}", band_id, e));
+                continue;
+            }
+        };
         let is_complete_str = if info.is_closed {
             "complete"
         } else {
@@ -93,28 +114,3 @@ pub fn show_entry_names<E: Entry, I: Iterator<Item = E>>(it: I, w: &mut dyn Writ
     }
     Ok(())
 }
-
-fn get_band_infos(archive: &Archive, sort_recent_first: bool) -> Result<Vec<band::Info>> {
-    let mut band_infos = vec![];
-    for band_id in archive.list_band_ids()? {
-        let band = match Band::open(&archive, &band_id) {
-            Ok(band) => band,
-            Err(e) => {
-                ui::problem(&format!("Failed to open band {:?}: {:?}", band_id, e));
-                continue;
-            }
-        };
-        let info = match band.get_info() {
-            Ok(info) => info,
-            Err(e) => {
-                ui::problem(&format!("Failed to read band tail {:?}: {:?}", band_id, e));
-                continue;
-            }
-        };
-        band_infos.push(info);
-    }
-    if sort_recent_first {
-        band_infos.sort_unstable_by_key(|info| std::cmp::Reverse(info.start_time));
-    }
-    Ok(band_infos)
-}

tests/cli.rs

@@ -483,3 +483,31 @@ fn size_exclude() {
         .success()
         .stdout("10\n");
 }
+
+#[test]
+fn brief_versions_sort_recent_first() {
+    let af = ScratchArchive::new();
+    af.store_two_versions();
+
+    run_conserve()
+        .args(&["versions", "--short", "--newest"])
+        .arg(af.path())
+        .assert()
+        .success()
+        .stderr(predicate::str::is_empty())
+        .stdout("b0001\nb0000\n");
+}
+
+#[test]
+fn verbose_versions_sort_recent_first() {
+    let af = ScratchArchive::new();
+    af.store_two_versions();
+
+    run_conserve()
+        .args(&["versions", "--newest"])
+        .arg(af.path())
+        .assert()
+        .success()
+        .stderr(predicate::str::is_empty())
+        .stdout(predicate::str::is_match("b0001.*\nb0000.*").unwrap());
+}