Introduce "sort specs" to allow fine-grained sorting of files in torrents

Sort specs are of the form `KEY:ORDER`, and allow sorting files in a
torrent by multiple criteria. Multiple sort specs can be passed with
`--sort-by` upon torrent creation.

type: added

src/common.rs

@@ -35,10 +35,7 @@ pub(crate) use serde_with::rust::unwrap_or_skip;
 pub(crate) use sha1::Sha1;
 pub(crate) use snafu::{ResultExt, Snafu};
 pub(crate) use static_assertions::const_assert;
-pub(crate) use structopt::{
+pub(crate) use structopt::{clap::AppSettings, StructOpt};
-  clap::{AppSettings, ArgSettings},
-  StructOpt,
-};
 pub(crate) use strum::VariantNames;
 pub(crate) use strum_macros::{EnumString, EnumVariantNames, IntoStaticStr};
 pub(crate) use unicode_width::UnicodeWidthStr;
@@ -61,15 +58,15 @@ pub(crate) use crate::{
 // structs and enums
 pub(crate) use crate::{
   arguments::Arguments, bytes::Bytes, env::Env, error::Error, file_error::FileError,
-  file_info::FileInfo, file_order::FileOrder, file_path::FilePath, file_status::FileStatus,
+  file_info::FileInfo, file_path::FilePath, file_status::FileStatus, files::Files, hasher::Hasher,
-  files::Files, hasher::Hasher, host_port::HostPort, host_port_parse_error::HostPortParseError,
+  host_port::HostPort, host_port_parse_error::HostPortParseError, info::Info, infohash::Infohash,
-  info::Info, infohash::Infohash, input::Input, input_target::InputTarget, lint::Lint,
+  input::Input, input_target::InputTarget, lint::Lint, linter::Linter, magnet_link::MagnetLink,
-  linter::Linter, magnet_link::MagnetLink, md5_digest::Md5Digest, metainfo::Metainfo,
+  md5_digest::Md5Digest, metainfo::Metainfo, metainfo_error::MetainfoError, mode::Mode,
-  metainfo_error::MetainfoError, mode::Mode, options::Options, output_stream::OutputStream,
+  options::Options, output_stream::OutputStream, output_target::OutputTarget,
-  output_target::OutputTarget, piece_length_picker::PieceLengthPicker, piece_list::PieceList,
+  piece_length_picker::PieceLengthPicker, piece_list::PieceList, platform::Platform,
-  platform::Platform, sha1_digest::Sha1Digest, status::Status, style::Style,
+  sha1_digest::Sha1Digest, sort_key::SortKey, sort_order::SortOrder, sort_spec::SortSpec,
-  subcommand::Subcommand, table::Table, torrent_summary::TorrentSummary, use_color::UseColor,
+  status::Status, style::Style, subcommand::Subcommand, table::Table,
-  verifier::Verifier, walker::Walker,
+  torrent_summary::TorrentSummary, use_color::UseColor, verifier::Verifier, walker::Walker,
 };
 
 // type aliases

src/file_order.rs

@@ -1,110 +0,0 @@
-use crate::common::*;
-
-#[derive(Eq, PartialEq, Debug, Copy, Clone, Ord, PartialOrd)]
-pub(crate) enum FileOrder {
-  AlphabeticalDesc,
-  AlphabeticalAsc,
-  SizeDesc,
-  SizeAsc,
-}
-
-impl FileOrder {
-  pub(crate) const ALPHABETICAL_ASC: &'static str = "alphabetical-asc";
-  pub(crate) const ALPHABETICAL_DESC: &'static str = "alphabetical-desc";
-  pub(crate) const SIZE_ASC: &'static str = "size-asc";
-  pub(crate) const SIZE_DESC: &'static str = "size-desc";
-  pub(crate) const VALUES: &'static [&'static str] = &[
-    Self::ALPHABETICAL_DESC,
-    Self::ALPHABETICAL_ASC,
-    Self::SIZE_DESC,
-    Self::SIZE_ASC,
-  ];
-
-  pub(crate) fn name(self) -> &'static str {
-    match self {
-      Self::AlphabeticalDesc => Self::ALPHABETICAL_DESC,
-      Self::AlphabeticalAsc => Self::ALPHABETICAL_ASC,
-      Self::SizeDesc => Self::SIZE_DESC,
-      Self::SizeAsc => Self::SIZE_ASC,
-    }
-  }
-
-  pub(crate) fn compare_file_info(self, a: &FileInfo, b: &FileInfo) -> Ordering {
-    match self {
-      Self::AlphabeticalAsc => a.path.cmp(&b.path),
-      Self::AlphabeticalDesc => a.path.cmp(&b.path).reverse(),
-      Self::SizeAsc => a.length.cmp(&b.length).then_with(|| a.path.cmp(&b.path)),
-      Self::SizeDesc => a
-        .length
-        .cmp(&b.length)
-        .reverse()
-        .then_with(|| a.path.cmp(&b.path)),
-    }
-  }
-}
-
-impl FromStr for FileOrder {
-  type Err = Error;
-
-  fn from_str(text: &str) -> Result<Self, Self::Err> {
-    match text.replace('_', "-").to_lowercase().as_str() {
-      Self::ALPHABETICAL_DESC => Ok(Self::AlphabeticalDesc),
-      Self::ALPHABETICAL_ASC => Ok(Self::AlphabeticalAsc),
-      Self::SIZE_DESC => Ok(Self::SizeDesc),
-      Self::SIZE_ASC => Ok(Self::SizeAsc),
-      _ => Err(Error::FileOrderUnknown {
-        text: text.to_string(),
-      }),
-    }
-  }
-}
-
-impl Display for FileOrder {
-  fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-    write!(f, "{}", self.name())
-  }
-}
-
-#[cfg(test)]
-mod tests {
-  use super::*;
-
-  #[test]
-  fn from_str_ok() {
-    assert_eq!(
-      FileOrder::AlphabeticalDesc,
-      "alphabetical_desc".parse().unwrap()
-    );
-
-    assert_eq!(
-      FileOrder::AlphabeticalDesc,
-      "alphabetical-desc".parse().unwrap()
-    );
-
-    assert_eq!(
-      FileOrder::AlphabeticalDesc,
-      "ALPHABETICAL-desc".parse().unwrap()
-    );
-  }
-
-  #[test]
-  fn convert() {
-    fn case(text: &str, value: FileOrder) {
-      assert_eq!(value, text.parse().unwrap());
-      assert_eq!(value.name(), text);
-    }
-
-    case("alphabetical-desc", FileOrder::AlphabeticalDesc);
-    case("alphabetical-asc", FileOrder::AlphabeticalAsc);
-    case("size-desc", FileOrder::SizeDesc);
-    case("size-asc", FileOrder::SizeAsc);
-  }
-
-  #[test]
-  fn from_str_err() {
-    assert_matches!(
-      "foo".parse::<FileOrder>(),
-      Err(Error::FileOrderUnknown { text }) if text == "foo"
-    );
-  }
-}

src/main.rs

@@ -56,7 +56,6 @@ mod env;
 mod error;
 mod file_error;
 mod file_info;
-mod file_order;
 mod file_path;
 mod file_status;
 mod files;
@@ -87,6 +86,9 @@ mod platform_interface;
 mod print;
 mod reckoner;
 mod sha1_digest;
+mod sort_key;
+mod sort_order;
+mod sort_spec;
 mod status;
 mod step;
 mod style;

src/sort_key.rs

@@ -0,0 +1,14 @@
+use crate::common::*;
+
+#[derive(Clone, Copy, Debug, PartialEq, IntoStaticStr, EnumString)]
+#[strum(serialize_all = "kebab-case")]
+pub(crate) enum SortKey {
+  Path,
+  Size,
+}
+
+impl SortKey {
+  pub(crate) fn name(self) -> &'static str {
+    self.into()
+  }
+}

src/sort_order.rs

@@ -0,0 +1,20 @@
+use crate::common::*;
+
+#[derive(Clone, Copy, Debug, PartialEq, IntoStaticStr, EnumString)]
+#[strum(serialize_all = "kebab-case")]
+pub(crate) enum SortOrder {
+  Ascending,
+  Descending,
+}
+
+impl SortOrder {
+  pub(crate) fn name(self) -> &'static str {
+    self.into()
+  }
+}
+
+impl Default for SortOrder {
+  fn default() -> Self {
+    Self::Ascending
+  }
+}

src/sort_spec.rs

@@ -0,0 +1,95 @@
+use crate::common::*;
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub(crate) struct SortSpec {
+  key: SortKey,
+  order: SortOrder,
+}
+
+impl SortSpec {
+  pub(crate) fn compare(specs: &[SortSpec], a: &FileInfo, b: &FileInfo) -> Ordering {
+    let mut specs = specs.to_vec();
+
+    specs.push(SortSpec::default());
+
+    Self::compare_specs(&specs, a, b)
+  }
+
+  fn compare_specs(specs: &[SortSpec], a: &FileInfo, b: &FileInfo) -> Ordering {
+    specs.iter().fold(Ordering::Equal, |ordering, spec| {
+      ordering.then_with(|| spec.compare_file_info(a, b))
+    })
+  }
+
+  fn compare_file_info(self, a: &FileInfo, b: &FileInfo) -> Ordering {
+    let ordering = match self.key {
+      SortKey::Path => a.path.cmp(&b.path),
+      SortKey::Size => a.length.cmp(&b.length),
+    };
+
+    match self.order {
+      SortOrder::Ascending => ordering,
+      SortOrder::Descending => ordering.reverse(),
+    }
+  }
+}
+
+impl Default for SortSpec {
+  fn default() -> Self {
+    Self {
+      key: SortKey::Path,
+      order: SortOrder::default(),
+    }
+  }
+}
+
+impl FromStr for SortSpec {
+  type Err = strum::ParseError;
+
+  fn from_str(text: &str) -> Result<Self, Self::Err> {
+    if let Some(index) = text.find(':') {
+      Ok(SortSpec {
+        key: text[..index].parse()?,
+        order: text[index + 1..].parse()?,
+      })
+    } else {
+      Ok(SortSpec {
+        key: text.parse()?,
+        order: SortOrder::default(),
+      })
+    }
+  }
+}
+
+impl Display for SortSpec {
+  fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+    write!(f, "{}:{}", self.key.name(), self.order.name())
+  }
+}
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+
+  #[test]
+  fn default() {
+    assert_eq!(
+      SortSpec::default(),
+      SortSpec {
+        key: SortKey::Path,
+        order: SortOrder::Ascending
+      }
+    );
+  }
+
+  #[test]
+  fn parse() {
+    assert_eq!(
+      SortSpec {
+        key: SortKey::Path,
+        order: SortOrder::Ascending
+      },
+      "path:ascending".parse().unwrap()
+    );
+  }
+}

src/subcommand/torrent/create.rs

@@ -61,10 +61,15 @@ pub(crate) struct Create {
     value_name = "NODE",
     help = "Add DHT bootstrap node `NODE` to torrent. `NODE` should be in the form `HOST:PORT`, \
             where `HOST` is a domain name, an IPv4 address, or an IPv6 address surrounded by \
-            brackets. May be given more than once to add multiple bootstrap nodes. Examples:
+            brackets. May be given more than once to add multiple bootstrap nodes.
-    `--node router.example.com:1337`
+
-    `--node 203.0.113.0:2290`
+Examples:
-    `--node [2001:db8:4275:7920:6269:7463:6f69:6e21]:8832`"
+
+    --node router.example.com:1337
+
+    --node 203.0.113.0:2290
+
+    --node [2001:db8:4275:7920:6269:7463:6f69:6e21]:8832"
   )]
   dht_nodes: Vec<HostPort>,
   #[structopt(
@@ -153,14 +158,27 @@ pub(crate) struct Create {
   )]
   open: bool,
   #[structopt(
-    long = "order",
+    long = "sort-by",
-    value_name = "ORDER",
+    value_name = "SPEC",
-    possible_values = FileOrder::VALUES,
+    help = "Set the order of files within a torrent. `SPEC` should be of the form `KEY:ORDER`, \
-    set(ArgSettings::CaseInsensitive),
+            with `KEY` being one of `path` or `size`, and `ORDER` being `ascending` or \
-    help = "Specify the file order within the torrent. \
+            `descending`. `:ORDER` defaults to `ascending` if omitted. The `--sort-by` flag may \
-            Defaults to ascending alphabetical order."
+            be given more than once, with later values being used to break ties. Ties that remain \
+            are broken in ascending path order.
+
+Sort in ascending order by path, the default:
+
+    --sort-by path:ascending
+
+Sort in ascending order by path, more concisely:
+
+    --sort-by path
+
+Sort in ascending order by size, break ties in descending path order:
+
+    --sort-by size:ascending --sort-by path:descending"
   )]
-  order: Option<FileOrder>,
+  sort_by: Vec<SortSpec>,
   #[structopt(
     long = "output",
     short = "o",
@@ -254,7 +272,7 @@ impl Create {
       .include_junk(self.include_junk)
       .include_hidden(self.include_hidden)
       .follow_symlinks(self.follow_symlinks)
-      .file_order(self.order.unwrap_or(FileOrder::AlphabeticalAsc))
+      .sort_by(self.sort_by)
       .globs(&self.globs)?
       .spinner(spinner)
       .files()?;
@@ -2423,15 +2441,15 @@ Content Size  9 bytes
   }
 
   #[test]
-  fn file_ordering_by_alpha_asc() {
+  fn file_ordering_by_path_ascending() {
     let mut env = test_env! {
       args: [
         "torrent",
         "create",
         "--input",
         "foo",
-        "--order",
+        "--sort-by",
-        "alphabetical-asc",
+        "path",
       ],
       tree: {
         foo: {
@@ -2452,15 +2470,15 @@ Content Size  9 bytes
   }
 
   #[test]
-  fn file_ordering_by_alpha_desc() {
+  fn file_ordering_by_path_descending() {
     let mut env = test_env! {
       args: [
         "torrent",
         "create",
         "--input",
         "foo",
-        "--order",
+        "--sort-by",
-        "alphabetical-desc",
+        "path:descending",
       ],
       tree: {
         foo: {
@@ -2481,15 +2499,15 @@ Content Size  9 bytes
   }
 
   #[test]
-  fn file_ordering_by_size_asc() {
+  fn file_ordering_by_size_ascending() {
     let mut env = test_env! {
       args: [
         "torrent",
         "create",
         "--input",
         "foo",
-        "--order",
+        "--sort-by",
-        "size-asc",
+        "size:ascending",
       ],
       tree: {
         foo: {
@@ -2510,15 +2528,15 @@ Content Size  9 bytes
   }
 
   #[test]
-  fn file_ordering_by_size_desc() {
+  fn file_ordering_by_size_descending() {
     let mut env = test_env! {
       args: [
         "torrent",
         "create",
         "--input",
         "foo",
-        "--order",
+        "--sort-by",
-        "size-desc",
+        "size:descending",
       ],
       tree: {
         foo: {
@@ -2537,4 +2555,35 @@ Content Size  9 bytes
     let torrent = env.load_metainfo("foo.torrent");
     assert_eq!(torrent.file_paths(), &["c", "a", "b", "d/e"]);
   }
+
+  #[test]
+  fn file_ordering_by_size_ascending_break_ties_path_descending() {
+    let mut env = test_env! {
+      args: [
+        "torrent",
+        "create",
+        "--input",
+        "foo",
+        "--sort-by",
+        "size:ascending",
+        "--sort-by",
+        "path:descending",
+      ],
+      tree: {
+        foo: {
+          a: "aa",
+          b: "b",
+          c: "ccc",
+          d: {
+            e: "e",
+          },
+        },
+      }
+    };
+
+    assert_matches!(env.run(), Ok(()));
+
+    let torrent = env.load_metainfo("foo.torrent");
+    assert_eq!(torrent.file_paths(), &["d/e", "b", "a", "c"]);
+  }
 }

src/walker.rs

@@ -12,7 +12,7 @@ pub(crate) struct Walker {
   follow_symlinks: bool,
   include_hidden: bool,
   include_junk: bool,
-  file_order: FileOrder,
+  sort_by: Vec<SortSpec>,
   patterns: Vec<Pattern>,
   root: PathBuf,
   spinner: Option<ProgressBar>,
@@ -24,7 +24,7 @@ impl Walker {
       follow_symlinks: false,
       include_hidden: false,
       include_junk: false,
-      file_order: FileOrder::AlphabeticalAsc,
+      sort_by: Vec::new(),
       patterns: Vec::new(),
       root: root.to_owned(),
       spinner: None,
@@ -45,8 +45,8 @@ impl Walker {
     }
   }
 
-  pub(crate) fn file_order(self, file_order: FileOrder) -> Self {
+  pub(crate) fn sort_by(self, sort_by: Vec<SortSpec>) -> Self {
-    Self { file_order, ..self }
+    Self { sort_by, ..self }
   }
 
   pub(crate) fn globs(mut self, globs: &[String]) -> Result<Self, Error> {
@@ -169,7 +169,7 @@ impl Walker {
       });
     }
 
-    file_infos.sort_by(|a, b| self.file_order.compare_file_info(a, b));
+    file_infos.sort_by(|a, b| SortSpec::compare(&self.sort_by, a, b));
 
     Ok(Files::dir(
       self.root,