From 97236128ea28093aee95f6536598bbaf48a3263e Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Mon, 3 Jul 2017 17:04:37 +0100 Subject: [PATCH] =?UTF-8?q?Only=20get=20an=20Env=20if=20one=E2=80=99s=20be?= =?UTF-8?q?ing=20used,=20also=20mutexes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit ties a table’s Environment to the fact that it contains columns. Previously, the Details view would get its Environment, and then use those fields to actually display the details in the table: except for the case where we’re only displaying a tree, when it would just be ignored, instead. This was caused by the “no columns” case using a Vec of no Columns behind the scenes, rather than disabling the table entirely; much like how a tap isn’t a zero-length swipe, the code should have been updated to reflect this. Now, the Environment is only created if it’s going to be used. Also, fix a double-mutex-lock: the mutable Table had to be accessed under a lock, but the table contained a UsersCache, which *also* had to be accessed under a lock. This was changed so that the table is only updated *after* the threads have all been joined, so there’s no need for any lock at all. May fix #141, but not sure. --- src/output/details.rs | 60 ++++++++++++++++++++++---------------- src/output/grid_details.rs | 4 +-- src/output/table.rs | 12 +++----- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/output/details.rs b/src/output/details.rs index 1d8a862..6fcf4a8 100644 --- a/src/output/details.rs +++ b/src/output/details.rs @@ -122,7 +122,7 @@ pub struct Render<'a> { struct Egg<'a> { - table_row: TableRow, + table_row: Option, xattrs: Vec, errors: Vec<(IOError, Option)>, dir: Option, @@ -138,24 +138,31 @@ impl<'a> AsRef> for Egg<'a> { impl<'a> Render<'a> { pub fn render(self, w: &mut W) -> IOResult<()> { - let columns_for_dir = match self.opts.columns { - Some(cols) => cols.for_dir(self.dir), - None => Vec::new(), - }; - - let env = Environment::default(); - let mut table = Table::new(&columns_for_dir, &self.colours, &env); let mut rows = Vec::new(); - if self.opts.header { - let header = table.header_row(); - rows.push(self.render_header(header)); + if let Some(columns) = self.opts.columns { + let env = Environment::default(); + let colz = columns.for_dir(self.dir); + let mut table = Table::new(&colz, &self.colours, &env); + + if self.opts.header { + let header = table.header_row(); + rows.push(self.render_header(header)); + } + + let mut table = Some(table); + self.add_files_to_table(&mut table, &mut rows, &self.files, 0); + + for row in self.iterate(table.as_ref(), rows) { + writeln!(w, "{}", row.strings())? + } } + else { + self.add_files_to_table(&mut None, &mut rows, &self.files, 0); - self.add_files_to_table(&mut table, &mut rows, &self.files, 0); - - for row in self.iterate(&table, rows) { - writeln!(w, "{}", row.strings())? + for row in self.iterate(None, rows) { + writeln!(w, "{}", row.strings())? + } } Ok(()) @@ -163,7 +170,7 @@ impl<'a> Render<'a> { /// Adds files to the table, possibly recursively. This is easily /// parallelisable, and uses a pool of threads. - fn add_files_to_table<'dir>(&self, mut table: &mut Table, rows: &mut Vec, src: &Vec>, depth: usize) { + fn add_files_to_table<'dir>(&self, table: &mut Option>, rows: &mut Vec, src: &Vec>, depth: usize) { use num_cpus; use scoped_threadpool::Pool; use std::sync::{Arc, Mutex}; @@ -174,11 +181,10 @@ impl<'a> Render<'a> { pool.scoped(|scoped| { let file_eggs = Arc::new(Mutex::new(&mut file_eggs)); - let table = Arc::new(Mutex::new(&mut table)); + let table = table.as_ref(); for file in src { let file_eggs = file_eggs.clone(); - let table = table.clone(); scoped.execute(move || { let mut errors = Vec::new(); @@ -191,7 +197,7 @@ impl<'a> Render<'a> { }; } - let table_row = table.lock().unwrap().row_for_file(&file, !xattrs.is_empty()); + let table_row = table.as_ref().map(|t| t.row_for_file(&file, !xattrs.is_empty())); if !self.opts.xattr { xattrs.clear(); @@ -220,9 +226,13 @@ impl<'a> Render<'a> { let mut files = Vec::new(); let mut errors = egg.errors; + if let (Some(ref mut t), Some(ref row)) = (table.as_mut(), egg.table_row.as_ref()) { + t.add_widths(row); + } + let row = Row { depth: depth, - cells: Some(egg.table_row), + cells: egg.table_row, name: FileName::new(&egg.file, LinkStyle::FullLinkPaths, self.classify, self.colours).paint().promote(), last: index == num_eggs - 1, }; @@ -307,10 +317,10 @@ impl<'a> Render<'a> { } /// Render the table as a vector of Cells, to be displayed on standard output. - pub fn iterate(&self, table: &'a Table<'a>, rows: Vec) -> Iter<'a> { + pub fn iterate(&self, table: Option<&'a Table<'a>>, rows: Vec) -> Iter<'a> { Iter { tree_trunk: TreeTrunk::default(), - total_width: table.columns_count() + table.widths().iter().fold(0, Add::add), + total_width: table.map(|t| t.columns_count() + t.widths().iter().fold(0, Add::add)).unwrap_or(0), table: table, inner: rows.into_iter(), colours: self.colours, @@ -319,7 +329,7 @@ impl<'a> Render<'a> { } pub struct Iter<'a> { - table: &'a Table<'a>, + table: Option<&'a Table<'a>>, tree_trunk: TreeTrunk, total_width: usize, colours: &'a Colours, @@ -332,8 +342,8 @@ impl<'a> Iterator for Iter<'a> { fn next(&mut self) -> Option { self.inner.next().map(|row| { let mut cell = - if let Some(cells) = row.cells { - self.table.render(cells) + if let (Some(table), Some(cells)) = (self.table, row.cells) { + table.render(cells) } else { let mut cell = TextCell::default(); diff --git a/src/output/grid_details.rs b/src/output/grid_details.rs index 5b6ac89..46bf6b1 100644 --- a/src/output/grid_details.rs +++ b/src/output/grid_details.rs @@ -50,7 +50,7 @@ impl<'a> Render<'a> { let drender = self.clone().details(); - let (mut first_table, _) = self.make_table(&env, &columns_for_dir, &drender); + let (first_table, _) = self.make_table(&env, &columns_for_dir, &drender); let rows = self.files.iter() .map(|file| first_table.row_for_file(file, file_has_xattrs(file))) @@ -122,7 +122,7 @@ impl<'a> Render<'a> { } let columns: Vec<_> = tables.into_iter().map(|(table, details_rows)| { - drender.iterate(&table, details_rows).collect::>() + drender.iterate(Some(&table), details_rows).collect::>() }).collect(); let direction = if self.grid.across { grid::Direction::LeftToRight } diff --git a/src/output/table.rs b/src/output/table.rs index 1b67893..9bfea37 100644 --- a/src/output/table.rs +++ b/src/output/table.rs @@ -109,14 +109,10 @@ impl<'a, 'f> Table<'a> { Row { cells } } - pub fn row_for_file(&mut self, file: &File, xattrs: bool) -> Row { - let mut cells = Vec::with_capacity(self.columns.len()); - - let other = self.columns.iter().map(|c| self.display(file, c, xattrs)).collect::>(); - for (old_width, column) in self.widths.iter_mut().zip(other.into_iter()) { - *old_width = max(*old_width, *column.width); - cells.push(column); - } + pub fn row_for_file(&self, file: &File, xattrs: bool) -> Row { + let cells = self.columns.iter() + .map(|c| self.display(file, c, xattrs)) + .collect(); Row { cells } }