PermissionsPlus holds the leftmost column values

The three pieces of information for the leftmost details view column (file type, permissions, and whether xattrs are present) used to be gathered from separate sources and passed around separately before being displayed at the end. Now, file type and permissions are put into a struct, along with the xattrs boolean that’s still getting passed around all over the place but not quite as much.

This was all done because I wanted to be able to test permissions rendering, without having file type and xattrs dragged into the same function.

src/fs/fields.rs

@@ -69,6 +69,15 @@ pub struct Permissions {
     pub other_execute:  bool,
 }
 
+/// The three pieces of information that are displayed as a single column in
+/// the details view. These values are fused together to make the output a
+/// little more compressed.
+pub struct PermissionsPlus {
+    pub file_type:   Type,
+    pub permissions: Permissions,
+    pub xattrs:      bool,
+}
+
 
 /// A file’s number of hard links on the filesystem.
 ///

src/fs/file.rs

@@ -10,26 +10,6 @@ use fs::dir::Dir;
 use fs::fields as f;
 
 
-#[allow(trivial_numeric_casts)]
-mod modes {
-    use libc;
-
-    pub type Mode = u32;
-    // The `libc::mode_t` type’s actual type varies, but the value returned
-    // from `metadata.permissions().mode()` is always `u32`.
-
-    pub const USER_READ: Mode     = libc::S_IRUSR as Mode;
-    pub const USER_WRITE: Mode    = libc::S_IWUSR as Mode;
-    pub const USER_EXECUTE: Mode  = libc::S_IXUSR as Mode;
-    pub const GROUP_READ: Mode    = libc::S_IRGRP as Mode;
-    pub const GROUP_WRITE: Mode   = libc::S_IWGRP as Mode;
-    pub const GROUP_EXECUTE: Mode = libc::S_IXGRP as Mode;
-    pub const OTHER_READ: Mode    = libc::S_IROTH as Mode;
-    pub const OTHER_WRITE: Mode   = libc::S_IWOTH as Mode;
-    pub const OTHER_EXECUTE: Mode = libc::S_IXOTH as Mode;
-}
-
-
 /// A **File** is a wrapper around one of Rust's Path objects, along with
 /// associated data about the file.
 ///
@@ -327,11 +307,7 @@ impl<'dir> File<'dir> {
         }
     }
 
-    /// This file's permissions, with flags for each bit.
-    ///
-    /// The extended-attribute '@' character that you see in here is in fact
-    /// added in later, to avoid querying the extended attributes more than
-    /// once. (Yes, it's a little hacky.)
+    /// This file’s permissions, with flags for each bit.
     pub fn permissions(&self) -> f::Permissions {
         let bits = self.metadata.permissions().mode();
         let has_bit = |bit| { bits & bit == bit };
@@ -449,6 +425,27 @@ impl<'dir> FileTarget<'dir> {
 }
 
 
+/// More readable aliases for the permission bits exposed by libc.
+#[allow(trivial_numeric_casts)]
+mod modes {
+    use libc;
+
+    pub type Mode = u32;
+    // The `libc::mode_t` type’s actual type varies, but the value returned
+    // from `metadata.permissions().mode()` is always `u32`.
+
+    pub const USER_READ: Mode     = libc::S_IRUSR as Mode;
+    pub const USER_WRITE: Mode    = libc::S_IWUSR as Mode;
+    pub const USER_EXECUTE: Mode  = libc::S_IXUSR as Mode;
+    pub const GROUP_READ: Mode    = libc::S_IRGRP as Mode;
+    pub const GROUP_WRITE: Mode   = libc::S_IWGRP as Mode;
+    pub const GROUP_EXECUTE: Mode = libc::S_IXGRP as Mode;
+    pub const OTHER_READ: Mode    = libc::S_IROTH as Mode;
+    pub const OTHER_WRITE: Mode   = libc::S_IWOTH as Mode;
+    pub const OTHER_EXECUTE: Mode = libc::S_IXOTH as Mode;
+}
+
+
 #[cfg(test)]
 mod test {
     use super::ext;

src/output/details.rs

@@ -492,11 +492,19 @@ impl<'a, U: Users+Groups+'a> Table<'a, U> {
                     .collect()
     }
 
+    fn permissions_plus(&self, file: &File, xattrs: bool) -> f::PermissionsPlus {
+        f::PermissionsPlus {
+            file_type: file.type_char(),
+            permissions: file.permissions(),
+            xattrs: xattrs,
+        }
+    }
+
     fn display(&self, file: &File, column: &Column, xattrs: bool) -> TextCell {
         use output::column::TimeType::*;
 
         match *column {
-            Column::Permissions          => file.permissions().render(&self.opts.colours, file.type_char(), xattrs),
+            Column::Permissions          => self.permissions_plus(file, xattrs).render(&self.opts.colours),
             Column::FileSize(fmt)        => file.size().render(&self.opts.colours, fmt, &self.env.numeric),
             Column::Timestamp(Modified)  => self.render_time(file.modified_time()),
             Column::Timestamp(Created)   => self.render_time(file.created_time()),

src/output/render/permissions.rs

@@ -4,42 +4,48 @@ use output::cell::{TextCell, DisplayWidth};
 use ansi_term::{ANSIString, Style};
 
 
-impl f::Permissions {
-    pub fn render(&self, colours: &Colours, file_type: f::Type, xattrs: bool) -> TextCell {
-       let bit = |bit, chr: &'static str, style: Style| {
-           if bit { style.paint(chr) } else { colours.punctuation.paint("-") }
-       };
-
-       let x_colour = if file_type.is_regular_file() { colours.perms.user_execute_file }
-                                                else { colours.perms.user_execute_other };
-
-       let mut chars = vec![
-           file_type.render(colours),
-           bit(self.user_read,     "r", colours.perms.user_read),
-           bit(self.user_write,    "w", colours.perms.user_write),
-           bit(self.user_execute,  "x", x_colour),
-           bit(self.group_read,    "r", colours.perms.group_read),
-           bit(self.group_write,   "w", colours.perms.group_write),
-           bit(self.group_execute, "x", colours.perms.group_execute),
-           bit(self.other_read,    "r", colours.perms.other_read),
-           bit(self.other_write,   "w", colours.perms.other_write),
-           bit(self.other_execute, "x", colours.perms.other_execute),
-       ];
-
-       if xattrs {
+impl f::PermissionsPlus {
+    pub fn render(&self, colours: &Colours) -> TextCell {
+        let x_colour = if self.file_type.is_regular_file() { colours.perms.user_execute_file }
+                                                      else { colours.perms.user_execute_other };
+
+        let mut chars = vec![ self.file_type.render(colours) ];
+        chars.extend(self.permissions.render(colours, x_colour));
+
+        if self.xattrs {
            chars.push(colours.perms.attribute.paint("@"));
-       }
-
-       // As these are all ASCII characters, we can guarantee that they’re
-       // all going to be one character wide, and don’t need to compute the
-       // cell’s display width.
-       let width = DisplayWidth::from(chars.len());
-
-       TextCell {
-           contents: chars.into(),
-           width:    width,
-       }
-   }
+        }
+
+        // As these are all ASCII characters, we can guarantee that they’re
+        // all going to be one character wide, and don’t need to compute the
+        // cell’s display width.
+        let width = DisplayWidth::from(chars.len());
+
+        TextCell {
+            contents: chars.into(),
+            width:    width,
+        }
+    }
+}
+
+impl f::Permissions {
+    pub fn render(&self, colours: &Colours, x_colour: Style) -> Vec<ANSIString<'static>> {
+        let bit = |bit, chr: &'static str, style: Style| {
+            if bit { style.paint(chr) } else { colours.punctuation.paint("-") }
+        };
+
+        vec![
+            bit(self.user_read,     "r", colours.perms.user_read),
+            bit(self.user_write,    "w", colours.perms.user_write),
+            bit(self.user_execute,  "x", x_colour),
+            bit(self.group_read,    "r", colours.perms.group_read),
+            bit(self.group_write,   "w", colours.perms.group_write),
+            bit(self.group_execute, "x", colours.perms.group_execute),
+            bit(self.other_read,    "r", colours.perms.other_read),
+            bit(self.other_write,   "w", colours.perms.other_write),
+            bit(self.other_execute, "x", colours.perms.other_execute),
+        ]
+    }
 }
 
 impl f::Type {
@@ -63,78 +69,59 @@ impl f::Type {
 #[allow(unused_results)]
 pub mod test {
     use output::details::Details;
-
+    use output::cell::TextCellContents;
     use fs::fields as f;
-    use output::cell::TextCell;
 
-    use users::{User, Group};
-    use users::mock::MockUsers;
-    use users::os::unix::GroupExt;
     use ansi_term::Colour::*;
 
 
     #[test]
-    fn named() {
+    fn negate() {
         let mut details = Details::default();
-        details.colours.users.group_not_yours = Fixed(101).normal();
+        details.colours.punctuation = Fixed(44).normal();
 
-        let mut users = MockUsers::with_current_uid(1000);
-        users.add_group(Group::new(100, "folk"));
+        let bits = f::Permissions {
+            user_read:  false,  user_write:  false,  user_execute:  false,
+            group_read: false,  group_write: false,  group_execute: false,
+            other_read: false,  other_write: false,  other_execute: false,
+        };
 
-        let group = f::Group(100);
-        let expected = TextCell::paint_str(Fixed(101).normal(), "folk");
-        assert_eq!(expected, group.render(&details.colours, &users))
-    }
+        let expected = TextCellContents::from(vec![
+            Fixed(44).paint("-"),  Fixed(44).paint("-"),  Fixed(44).paint("-"),
+            Fixed(44).paint("-"),  Fixed(44).paint("-"),  Fixed(44).paint("-"),
+            Fixed(44).paint("-"),  Fixed(44).paint("-"),  Fixed(44).paint("-"),
+        ]);
 
-    #[test]
-    fn unnamed() {
-        let mut details = Details::default();
-        details.colours.users.group_not_yours = Fixed(87).normal();
-
-        let users = MockUsers::with_current_uid(1000);
-
-        let group = f::Group(100);
-        let expected = TextCell::paint_str(Fixed(87).normal(), "100");
-        assert_eq!(expected, group.render(&details.colours, &users));
+        assert_eq!(expected, bits.render(&details.colours, Fixed(66).normal()).into())
     }
 
-    #[test]
-    fn primary() {
-        let mut details = Details::default();
-        details.colours.users.group_yours = Fixed(64).normal();
-
-        let mut users = MockUsers::with_current_uid(2);
-        users.add_user(User::new(2, "eve", 100));
-        users.add_group(Group::new(100, "folk"));
-
-        let group = f::Group(100);
-        let expected = TextCell::paint_str(Fixed(64).normal(), "folk");
-        assert_eq!(expected, group.render(&details.colours, &users))
-    }
 
     #[test]
-    fn secondary() {
+    fn affirm() {
         let mut details = Details::default();
-        details.colours.users.group_yours = Fixed(31).normal();
-
-        let mut users = MockUsers::with_current_uid(2);
-        users.add_user(User::new(2, "eve", 666));
-
-        let test_group = Group::new(100, "folk").add_member("eve");
-        users.add_group(test_group);
-
-        let group = f::Group(100);
-        let expected = TextCell::paint_str(Fixed(31).normal(), "folk");
-        assert_eq!(expected, group.render(&details.colours, &users))
-    }
-
-    #[test]
-    fn overflow() {
-        let mut details = Details::default();
-        details.colours.users.group_not_yours = Blue.underline();
-
-        let group = f::Group(2_147_483_648);
-        let expected = TextCell::paint_str(Blue.underline(), "2147483648");
-        assert_eq!(expected, group.render(&details.colours, &MockUsers::with_current_uid(0)));
+        details.colours.perms.user_read    = Fixed(101).normal();
+        details.colours.perms.user_write   = Fixed(102).normal();
+
+        details.colours.perms.group_read    = Fixed(104).normal();
+        details.colours.perms.group_write   = Fixed(105).normal();
+        details.colours.perms.group_execute = Fixed(106).normal();
+
+        details.colours.perms.other_read    = Fixed(107).normal();
+        details.colours.perms.other_write   = Fixed(108).normal();
+        details.colours.perms.other_execute = Fixed(109).normal();
+
+        let bits = f::Permissions {
+            user_read:  true,  user_write:  true,  user_execute:  true,
+            group_read: true,  group_write: true,  group_execute: true,
+            other_read: true,  other_write: true,  other_execute: true,
+        };
+
+        let expected = TextCellContents::from(vec![
+            Fixed(101).paint("r"),  Fixed(102).paint("w"),  Fixed(103).paint("x"),
+            Fixed(104).paint("r"),  Fixed(105).paint("w"),  Fixed(106).paint("x"),
+            Fixed(107).paint("r"),  Fixed(108).paint("w"),  Fixed(109).paint("x"),
+        ]);
+
+        assert_eq!(expected, bits.render(&details.colours, Fixed(103).normal()).into())
     }
 }