Print xattrs in tree view like we do errors

This changes the way extended attributes (xattrs) are printed. Before, they were artificially printed out on their own line both in lines mode *and* details mode, which looked a bit weird. Now, they are additional 'child nodes' of that item that get printed alongside errors.

All this allows all the 'extra info' that is going to be present for very few entries to be consolidated and listed in the same way, without resorting to extra printlns.

As a great side-effect, it allows taking out some of the more redundant code in the Table impl -- it is now *always* going to be in create-child-nodes mode, as *any* file now can, not only when we have the --tree flag in use.

Also, it now actually displays errors when failing to read the extended attributes, such as if the user doesn't have permission to read them.

The extended attribute flag has been temporarily disabled while I work out the best way to do it!

src/feature/xattr.rs

@@ -39,23 +39,29 @@ pub struct Attribute {
 pub fn list_attrs(lister: lister::Lister, path: &Path) -> io::Result<Vec<Attribute>> {
     let c_path = match path.as_os_str().to_cstring() {
         Some(cstring) => cstring,
-        None => return Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes")),
+        None => return Err(io::Error::new(io::ErrorKind::Other, "Error: path somehow contained a NUL?")),
     };
 
+    let mut names = Vec::new();
     let bufsize = lister.listxattr_first(&c_path);
 
-    if bufsize > 0 {
+    if bufsize < 0 {
+        return Err(io::Error::last_os_error());
+    }
+    else if bufsize > 0 {
         let mut buf = vec![0u8; bufsize as usize];
         let err = lister.listxattr_second(&c_path, &mut buf, bufsize);
 
+        if err < 0 {
+            return Err(io::Error::last_os_error());
+        }
+
         if err > 0 {
             // End indicies of the attribute names
             // the buffer contains 0-terminates c-strings
             let idx = buf.iter().enumerate().filter_map(|(i, v)|
                 if *v == 0 { Some(i) } else { None }
             );
-
-            let mut names = Vec::new();
             let mut start = 0;
 
             for end in idx {
@@ -72,15 +78,10 @@ pub fn list_attrs(lister: lister::Lister, path: &Path) -> io::Result<Vec<Attribu
                 start = c_end;
             }
 
-            Ok(names)
         }
-        else {
-            Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
-        }
-    }
-    else {
-        Err(io::Error::new(io::ErrorKind::Other, "could not read extended attributes"))
+
     }
+    Ok(names)
 }
 
 #[cfg(target_os = "macos")]

src/file.rs

@@ -12,7 +12,6 @@ use unicode_width::UnicodeWidthStr;
 
 use dir::Dir;
 use options::TimeType;
-use feature::xattr::{Attribute, FileAttributes};
 
 use self::fields as f;
 
@@ -45,10 +44,6 @@ pub struct File<'dir> {
     /// cache it.
     pub metadata: fs::Metadata,
 
-    /// List of this file's extended attributes. These are only loaded if the
-    /// `xattr` feature is in use.
-    pub xattrs: Vec<Attribute>,
-
     /// A reference to the directory that contains this file, if present.
     ///
     /// Filenames that get passed in on the command-line directly will have no
@@ -93,7 +88,6 @@ impl<'dir> File<'dir> {
             dir:    parent,
             metadata:   metadata,
             ext:    ext(&filename),
-            xattrs: path.symlink_attributes().unwrap_or(Vec::new()),
             name:   filename.to_string(),
             this:   this,
         }
@@ -199,7 +193,6 @@ impl<'dir> File<'dir> {
                 dir:    self.dir,
                 metadata:   metadata,
                 ext:    ext(&filename),
-                xattrs: target_path.attributes().unwrap_or(Vec::new()),
                 name:   filename.to_string(),
                 this:   None,
             })
@@ -316,7 +309,7 @@ impl<'dir> File<'dir> {
             other_read:     has_bit(unix::fs::OTHER_READ),
             other_write:    has_bit(unix::fs::OTHER_WRITE),
             other_execute:  has_bit(unix::fs::OTHER_EXECUTE),
-            attribute:      !self.xattrs.is_empty()
+            attribute:      false, // !self.xattrs.is_empty()
         }
     }
 

src/output/details.rs

@@ -1,13 +1,12 @@
 use std::error::Error;
 use std::io;
-use std::iter::repeat;
 use std::path::Path;
 use std::string::ToString;
 
 use colours::Colours;
 use column::{Alignment, Column, Cell};
 use dir::Dir;
-use feature::xattr::Attribute;
+use feature::xattr::{Attribute, FileAttributes};
 use file::fields as f;
 use file::File;
 use options::{Columns, FileFilter, RecurseOptions, SizeFormat};
@@ -78,7 +77,7 @@ impl Details {
 
         // Then add files to the table and print it out.
         self.add_files_to_table(&mut table, files, 0);
-        for cell in table.print_table(self.xattr, self.recurse.is_some()) {
+        for cell in table.print_table() {
             println!("{}", cell.text);
         }
     }
@@ -89,6 +88,17 @@ impl Details {
         for (index, file) in src.iter().enumerate() {
             table.add_file(file, depth, index == src.len() - 1, true);
 
+            if self.xattr {
+                match file.path.attributes() {
+                    Ok(xattrs) => {
+                        for xattr in xattrs {
+                            table.add_xattr(xattr, depth + 1, false);
+                        }
+                    },
+                    Err(e) => table.add_error(&e, depth + 1, true, None),
+                }
+            }
+
             // There are two types of recursion that exa supports: a tree
             // view, which is dealt with here, and multiple listings, which is
             // dealt with in the main module. So only actually recurse if we
@@ -149,16 +159,9 @@ struct Row {
     /// on top have depth 0.
     depth: usize,
 
-    /// Vector of this file's extended attributes, if that feature is active.
-    attrs: Vec<Attribute>,
-
     /// Whether this is the last entry in the directory. This flag is used
     /// when calculating the tree view.
     last: bool,
-
-    /// Whether this file is a directory and has any children. Also used when
-    /// calculating the tree view.
-    children: bool,
 }
 
 impl Row {
@@ -232,8 +235,6 @@ impl<U> Table<U> where U: Users {
             cells:    Some(self.columns.iter().map(|c| Cell::paint(self.colours.header, c.header())).collect()),
             name:     Cell::paint(self.colours.header, "Name"),
             last:     false,
-            attrs:    Vec::new(),
-            children: false,
         };
 
         self.rows.push(row);
@@ -250,8 +251,17 @@ impl<U> Table<U> where U: Users {
             cells:    None,
             name:     Cell::paint(self.colours.broken_arrow, &error_message),
             last:     last,
-            attrs:    Vec::new(),
-            children: false,
+        };
+
+        self.rows.push(row);
+    }
+
+    pub fn add_xattr(&mut self, xattr: Attribute, depth: usize, last: bool) {
+        let row = Row {
+            depth:    depth,
+            cells:    None,
+            name:     Cell::paint(self.colours.perms.attribute, &format!("{}\t{}", xattr.name, xattr.size)),
+            last:     last,
         };
 
         self.rows.push(row);
@@ -269,8 +279,6 @@ impl<U> Table<U> where U: Users {
             cells:    Some(cells),
             name:     Cell { text: filename(file, &self.colours, links), length: file.file_name_width() },
             last:     last,
-            attrs:    file.xattrs.clone(),
-            children: file.this.is_some(),
         };
 
         self.rows.push(row);
@@ -445,7 +453,7 @@ impl<U> Table<U> where U: Users {
     }
 
     /// Render the table as a vector of Cells, to be displayed on standard output.
-    pub fn print_table(&self, xattr: bool, show_children: bool) -> Vec<Cell> {
+    pub fn print_table(&self) -> Vec<Cell> {
         let mut stack = Vec::new();
         let mut cells = Vec::new();
 
@@ -482,40 +490,27 @@ impl<U> Table<U> where U: Users {
             // necessary to maintain information about the previously-printed
             // lines, as the output will change based on whether the
             // *previous* entry was the last in its directory.
-            if show_children {
-                stack.resize(row.depth + 1, TreePart::Edge);
-                stack[row.depth] = if row.last { TreePart::Corner } else { TreePart::Edge };
+            stack.resize(row.depth + 1, TreePart::Edge);
+            stack[row.depth] = if row.last { TreePart::Corner } else { TreePart::Edge };
 
-                for i in 1 .. row.depth + 1 {
-                    filename.push_str(&*self.colours.punctuation.paint(stack[i].ascii_art()).to_string());
-                    filename_length += 4;
-                }
+            for i in 1 .. row.depth + 1 {
+                filename.push_str(&*self.colours.punctuation.paint(stack[i].ascii_art()).to_string());
+                filename_length += 4;
+            }
 
-                if row.children {
-                    stack[row.depth] = if row.last { TreePart::Blank } else { TreePart::Line };
-                }
+            stack[row.depth] = if row.last { TreePart::Blank } else { TreePart::Line };
 
-                // If any tree characters have been printed, then add an extra
-                // space, which makes the output look much better.
-                if row.depth != 0 {
-                    filename.push(' ');
-                    filename_length += 1;
-                }
+            // If any tree characters have been printed, then add an extra
+            // space, which makes the output look much better.
+            if row.depth != 0 {
+                filename.push(' ');
+                filename_length += 1;
             }
 
             // Print the name without worrying about padding.
             filename.push_str(&*row.name.text);
             filename_length += row.name.length;
 
-            if xattr {
-                let width = row.attrs.iter().map(|a| a.name.len()).max().unwrap_or(0);
-                for attr in row.attrs.iter() {
-                    let name = &*attr.name;
-                    let spaces: String = repeat(" ").take(width - name.len()).collect();
-                    filename.push_str(&*format!("\n{}{}  {}", name, spaces, attr.size))
-                }
-            }
-
             cell.append(&Cell { text: filename, length: filename_length });
             cells.push(cell);
         }

src/output/grid_details.rs

@@ -73,7 +73,7 @@ impl GridDetails {
             tables[index].add_file_with_cells(row, file, 0, false, false);
         }
 
-        let columns: Vec<_> = tables.iter().map(|t| t.print_table(false, false)).collect();
+        let columns: Vec<_> = tables.iter().map(|t| t.print_table()).collect();
 
         let direction = if self.grid.across { grid::Direction::LeftToRight }
                                        else { grid::Direction::TopToBottom };