From dd8bff083fffd9a1f0610545617390f76176c505 Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Wed, 28 Jun 2017 18:41:31 +0100 Subject: [PATCH] Override the names of . and .. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was a problem when displaying . and .. in directory listings: their names would normalise to actual names! So instead of literally seeing `.`, you’d see the current directory’s name, inserted in sort order into the list of results. Obviously this is not what we want. In unrelated news, putting `.` and `..` into the list of paths read from a directory just takes up more heap space for something that’s basically constant. We can solve both these problems at once by moving the DotFilter to the files iterator in Dir, rather than at the Dir’s creation. Having the iterator know whether it should display `.` and `..` means it can emit those files first, and because it knows what those files really represent, it can override their file names to actually be those sequences of dots. This is not a perfect solution: the main casualty is that a File can now be constructed with a name, some metadata, both, or neither. This is currently handled with a bunch of Options, and returns IOResult even without doing any IO operations. But at least all the tests pass! --- Vagrantfile | 30 ++++++++++++ src/exa.rs | 8 +-- src/fs/dir.rs | 111 ++++++++++++++++++++++++++++++++++++------ src/fs/file.rs | 48 +++++++++--------- src/output/details.rs | 4 +- xtests/hiddens | 1 + xtests/hiddens_a | 1 + xtests/hiddens_aa | 1 + xtests/hiddens_l | 1 + xtests/hiddens_la | 3 ++ xtests/hiddens_laa | 5 ++ xtests/run.sh | 14 +++++- 12 files changed, 183 insertions(+), 44 deletions(-) create mode 100644 xtests/hiddens create mode 100644 xtests/hiddens_a create mode 100644 xtests/hiddens_aa create mode 100644 xtests/hiddens_l create mode 100644 xtests/hiddens_la create mode 100644 xtests/hiddens_laa diff --git a/Vagrantfile b/Vagrantfile index 91d4295..a10aaf5 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -369,6 +369,36 @@ Vagrant.configure(2) do |config| EOF + # Hidden and dot file testcases. + # We need to set the permissions of `.` and `..` because they actually + # get displayed in the output here, so this has to come last. + config.vm.provision :shell, privileged: false, inline: <<-EOF + set -xe + shopt -u dotglob + GLOBIGNORE=".:.." + + mkdir "#{test_dir}/hiddens" + touch "#{test_dir}/hiddens/visible" + touch "#{test_dir}/hiddens/.hidden" + touch "#{test_dir}/hiddens/..extra-hidden" + + # ./hiddens/ + touch -t #{some_date} "#{test_dir}/hiddens/"* + chmod 644 "#{test_dir}/hiddens/"* + sudo chown #{user}:#{user} "#{test_dir}/hiddens/"* + + # . + touch -t #{some_date} "#{test_dir}/hiddens" + chmod 755 "#{test_dir}/hiddens" + sudo chown #{user}:#{user} "#{test_dir}/hiddens" + + # .. + sudo touch -t #{some_date} "#{test_dir}" + sudo chmod 755 "#{test_dir}" + sudo chown #{user}:#{user} "#{test_dir}" + EOF + + # Install kcov for test coverage # This doesn’t run coverage over the xtests so it’s less useful for now if ENV.key?('INSTALL_KCOV') diff --git a/src/exa.rs b/src/exa.rs index aecf717..0376cdb 100644 --- a/src/exa.rs +++ b/src/exa.rs @@ -75,14 +75,14 @@ impl<'w, W: Write + 'w> Exa<'w, W> { } for file_name in &self.args { - match File::from_path(Path::new(&file_name), None) { + match File::new(Path::new(&file_name), None, None, None) { Err(e) => { exit_status = 2; writeln!(stderr(), "{}: {}", file_name, e)?; }, Ok(f) => { if f.is_directory() && !self.options.dir_action.treat_dirs_as_files() { - match f.to_dir(self.options.filter.dot_filter, self.options.should_scan_for_git()) { + match f.to_dir(self.options.should_scan_for_git()) { Ok(d) => dirs.push(d), Err(e) => writeln!(stderr(), "{}: {}", file_name, e)?, } @@ -126,7 +126,7 @@ impl<'w, W: Write + 'w> Exa<'w, W> { } let mut children = Vec::new(); - for file in dir.files() { + for file in dir.files(self.options.filter.dot_filter) { match file { Ok(file) => children.push(file), Err((path, e)) => writeln!(stderr(), "[{}: {}]", path.display(), e)?, @@ -142,7 +142,7 @@ impl<'w, W: Write + 'w> Exa<'w, W> { let mut child_dirs = Vec::new(); for child_dir in children.iter().filter(|f| f.is_directory()) { - match child_dir.to_dir(self.options.filter.dot_filter, false) { + match child_dir.to_dir(false) { Ok(d) => child_dirs.push(d), Err(e) => writeln!(stderr(), "{}: {}", child_dir.path.display(), e)?, } diff --git a/src/fs/dir.rs b/src/fs/dir.rs index 9314e9e..e9da9ba 100644 --- a/src/fs/dir.rs +++ b/src/fs/dir.rs @@ -36,18 +36,10 @@ impl Dir { /// The `read_dir` iterator doesn’t actually yield the `.` and `..` /// entries, so if the user wants to see them, we’ll have to add them /// ourselves after the files have been read. - pub fn read_dir(path: PathBuf, dots: DotFilter, git: bool) -> IOResult { - let mut contents: Vec = try!(fs::read_dir(&path)? + pub fn read_dir(path: PathBuf, git: bool) -> IOResult { + let contents: Vec = try!(fs::read_dir(&path)? .map(|result| result.map(|entry| entry.path())) .collect()); - match dots { - DotFilter::JustFiles => contents.retain(|p| p.file_name().and_then(|name| name.to_str()).map(|s| !s.starts_with('.')).unwrap_or(true)), - DotFilter::Dotfiles => {/* Don’t add or remove anything */}, - DotFilter::DotfilesAndDots => { - contents.insert(0, path.join("..")); - contents.insert(0, path.join(".")); - } - } let git = if git { Git::scan(&path).ok() } else { None }; Ok(Dir { contents, path, git }) @@ -55,10 +47,12 @@ impl Dir { /// Produce an iterator of IO results of trying to read all the files in /// this directory. - pub fn files(&self) -> Files { + pub fn files(&self, dots: DotFilter) -> Files { Files { - inner: self.contents.iter(), - dir: self, + inner: self.contents.iter(), + dir: self, + dotfiles: dots.shows_dotfiles(), + dots: dots.dots(), } } @@ -90,15 +84,83 @@ impl Dir { /// Iterator over reading the contents of a directory as `File` objects. pub struct Files<'dir> { + + /// The internal iterator over the paths that have been read already. inner: SliceIter<'dir, PathBuf>, + + /// The directory that begat those paths. dir: &'dir Dir, + + /// Whether to include dotfiles in the list. + dotfiles: bool, + + /// Whether the `.` or `..` directories should be produced first, before + /// any files have been listed. + dots: Dots, } +impl<'dir> Files<'dir> { + fn parent(&self) -> PathBuf { + // We can’t use `Path#parent` here because all it does is remove the + // last path component, which is no good for us if the path is + // relative. For example, while the parent of `/testcases/files` is + // `/testcases`, the parent of `.` is an empty path. Adding `..` on + // the end is the only way to get to the *actual* parent directory. + self.dir.path.join("..") + } + + /// Go through the directory until we encounter a file we can list (which + /// varies depending on the dotfile visibility flag) + fn next_visible_file(&mut self) -> Option, (PathBuf, io::Error)>> { + use fs::file::path_filename; + + loop { + if let Some(path) = self.inner.next() { + let filen = path_filename(path); + if !self.dotfiles && filen.starts_with(".") { continue } + + return Some(File::new(path, Some(self.dir), Some(filen), None) + .map_err(|e| (path.clone(), e))) + } + else { + return None + } + } + } +} + +/// The dot directories that need to be listed before actual files, if any. +/// If these aren’t being printed, then `FilesNext` is used to skip them. +enum Dots { + + /// List the `.` directory next. + DotNext, + + /// List the `..` directory next. + DotDotNext, + + /// Forget about the dot directories and just list files. + FilesNext, +} + + impl<'dir> Iterator for Files<'dir> { type Item = Result, (PathBuf, io::Error)>; fn next(&mut self) -> Option { - self.inner.next().map(|path| File::from_path(path, Some(self.dir)).map_err(|t| (path.clone(), t))) + if let Dots::DotNext = self.dots { + self.dots = Dots::DotDotNext; + Some(File::new(&self.dir.path, Some(self.dir), Some(String::from(".")), None) + .map_err(|e| (Path::new(".").to_path_buf(), e))) + } + else if let Dots::DotDotNext = self.dots { + self.dots = Dots::FilesNext; + Some(File::new(&self.parent(), Some(self.dir), Some(String::from("..")), None) + .map_err(|e| (self.parent(), e))) + } + else { + self.next_visible_file() + } } } @@ -124,3 +186,24 @@ impl Default for DotFilter { DotFilter::JustFiles } } + +impl DotFilter { + + /// Whether this filter should show dotfiles in a listing. + fn shows_dotfiles(&self) -> bool { + match *self { + DotFilter::JustFiles => false, + DotFilter::Dotfiles => true, + DotFilter::DotfilesAndDots => true, + } + } + + /// Whether this filter should add dot directories to a listing. + fn dots(&self) -> Dots { + match *self { + DotFilter::JustFiles => Dots::FilesNext, + DotFilter::Dotfiles => Dots::FilesNext, + DotFilter::DotfilesAndDots => Dots::DotNext, + } + } +} diff --git a/src/fs/file.rs b/src/fs/file.rs index 853109d..3d32c38 100644 --- a/src/fs/file.rs +++ b/src/fs/file.rs @@ -6,7 +6,7 @@ use std::io::Result as IOResult; use std::os::unix::fs::{MetadataExt, PermissionsExt, FileTypeExt}; use std::path::{Path, PathBuf}; -use fs::dir::{Dir, DotFilter}; +use fs::dir::Dir; use fs::fields as f; @@ -56,31 +56,33 @@ pub struct File<'dir> { pub dir: Option<&'dir Dir>, } -impl<'dir> File<'dir> { - - /// Create a new `File` object from the given `Path`, inside the given - /// `Dir`, if appropriate. - /// - /// This uses `symlink_metadata` instead of `metadata`, which doesn't - /// follow symbolic links. - pub fn from_path(path: &Path, parent: Option<&'dir Dir>) -> IOResult> { - fs::symlink_metadata(path).map(|metadata| File::with_metadata(metadata, path, parent)) +/// A file’s name is derived from its string. This needs to handle directories +/// such as `/` or `..`, which have no `file_name` component. So instead, just +/// use the last component as the name. +pub fn path_filename(path: &Path) -> String { + match path.components().next_back() { + Some(back) => back.as_os_str().to_string_lossy().to_string(), + None => path.display().to_string(), // use the path as fallback } +} - /// Create a new File object from the given metadata result, and other data. - pub fn with_metadata(metadata: fs::Metadata, path: &Path, parent: Option<&'dir Dir>) -> File<'dir> { - let filename = match path.components().next_back() { - Some(comp) => comp.as_os_str().to_string_lossy().to_string(), - None => String::new(), - }; +impl<'dir> File<'dir> { + pub fn new(path: &Path, parent: Option<&'dir Dir>, mut filename: Option, mut metadata: Option) -> IOResult> { + if filename.is_none() { + filename = Some(path_filename(path)); + } - File { + if metadata.is_none() { + metadata = Some(fs::symlink_metadata(path)?); + } + + Ok(File { path: path.to_path_buf(), dir: parent, - metadata: metadata, + metadata: metadata.unwrap(), ext: ext(path), - name: filename, - } + name: filename.unwrap(), + }) } /// Whether this file is a directory on the filesystem. @@ -94,8 +96,8 @@ impl<'dir> File<'dir> { /// /// Returns an IO error upon failure, but this shouldn't be used to check /// if a `File` is a directory or not! For that, just use `is_directory()`. - pub fn to_dir(&self, dots: DotFilter, scan_for_git: bool) -> IOResult { - Dir::read_dir(self.path.clone(), dots, scan_for_git) + pub fn to_dir(&self, scan_for_git: bool) -> IOResult { + Dir::read_dir(self.path.clone(), scan_for_git) } /// Whether this file is a regular file on the filesystem - that is, not a @@ -182,7 +184,7 @@ impl<'dir> File<'dir> { // Use plain `metadata` instead of `symlink_metadata` - we *want* to // follow links. if let Ok(metadata) = fs::metadata(&target_path) { - FileTarget::Ok(File::with_metadata(metadata, &*display_path, None)) + FileTarget::Ok(File::new(&*display_path, None, None, Some(metadata)).unwrap()) } else { FileTarget::Broken(display_path) diff --git a/src/output/details.rs b/src/output/details.rs index 405fe83..f514a46 100644 --- a/src/output/details.rs +++ b/src/output/details.rs @@ -315,7 +315,7 @@ impl<'a> Render<'a> { if let Some(r) = self.recurse { if file.is_directory() && r.tree && !r.is_too_deep(depth) { - if let Ok(d) = file.to_dir(self.filter.dot_filter, false) { + if let Ok(d) = file.to_dir(false) { dir = Some(d); } } @@ -344,7 +344,7 @@ impl<'a> Render<'a> { table.rows.push(row); if let Some(ref dir) = egg.dir { - for file_to_add in dir.files() { + for file_to_add in dir.files(self.filter.dot_filter) { match file_to_add { Ok(f) => files.push(f), Err((path, e)) => errors.push((e, Some(path))) diff --git a/xtests/hiddens b/xtests/hiddens new file mode 100644 index 0000000..9afbc9c --- /dev/null +++ b/xtests/hiddens @@ -0,0 +1 @@ +visible diff --git a/xtests/hiddens_a b/xtests/hiddens_a new file mode 100644 index 0000000..5300dfd --- /dev/null +++ b/xtests/hiddens_a @@ -0,0 +1 @@ +..extra-hidden .hidden visible diff --git a/xtests/hiddens_aa b/xtests/hiddens_aa new file mode 100644 index 0000000..f2b5c68 --- /dev/null +++ b/xtests/hiddens_aa @@ -0,0 +1 @@ +. .. ..extra-hidden .hidden visible diff --git a/xtests/hiddens_l b/xtests/hiddens_l new file mode 100644 index 0000000..8e6e048 --- /dev/null +++ b/xtests/hiddens_l @@ -0,0 +1 @@ +.rw-r--r-- 0 cassowary  1 Jan 12:34 visible diff --git a/xtests/hiddens_la b/xtests/hiddens_la new file mode 100644 index 0000000..c8d8f9d --- /dev/null +++ b/xtests/hiddens_la @@ -0,0 +1,3 @@ +.rw-r--r-- 0 cassowary  1 Jan 12:34 ..extra-hidden +.rw-r--r-- 0 cassowary  1 Jan 12:34 .hidden +.rw-r--r-- 0 cassowary  1 Jan 12:34 visible diff --git a/xtests/hiddens_laa b/xtests/hiddens_laa new file mode 100644 index 0000000..de014cb --- /dev/null +++ b/xtests/hiddens_laa @@ -0,0 +1,5 @@ +drwxr-xr-x - cassowary  1 Jan 12:34 . +drwxr-xr-x - cassowary  1 Jan 12:34 .. +.rw-r--r-- 0 cassowary  1 Jan 12:34 ..extra-hidden +.rw-r--r-- 0 cassowary  1 Jan 12:34 .hidden +.rw-r--r-- 0 cassowary  1 Jan 12:34 visible diff --git a/xtests/run.sh b/xtests/run.sh index 4e36285..8583909 100755 --- a/xtests/run.sh +++ b/xtests/run.sh @@ -38,7 +38,7 @@ COLUMNS=120 $exa $testcases/files | diff -q - $results/files_120 || exit 1 COLUMNS=160 $exa $testcases/files | diff -q - $results/files_160 || exit 1 COLUMNS=200 $exa $testcases/files | diff -q - $results/files_200 || exit 1 -COLUMNS=100 $exa $testcases/files/* | diff -q - $results/files_star_100 || exit 1 +COLUMNS=100 $exa $testcases/files/* | diff -q - $results/files_star_100 || exit 1 COLUMNS=150 $exa $testcases/files/* | diff -q - $results/files_star_150 || exit 1 COLUMNS=200 $exa $testcases/files/* | diff -q - $results/files_star_200 || exit 1 @@ -120,8 +120,20 @@ COLUMNS=80 $exa_binary --colour=automatic $testcases/files -l | diff -q - $resul $exa $testcases/git/additions -l --git 2>&1 | diff -q - $results/git_additions || exit 1 $exa $testcases/git/edits -l --git 2>&1 | diff -q - $results/git_edits || exit 1 + +# Hidden files +COLUMNS=80 $exa $testcases/hiddens 2>&1 | diff -q - $results/hiddens || exit 1 +COLUMNS=80 $exa $testcases/hiddens -a 2>&1 | diff -q - $results/hiddens_a || exit 1 +COLUMNS=80 $exa $testcases/hiddens -aa 2>&1 | diff -q - $results/hiddens_aa || exit 1 + +$exa $testcases/hiddens -l 2>&1 | diff -q - $results/hiddens_l || exit 1 +$exa $testcases/hiddens -l -a 2>&1 | diff -q - $results/hiddens_la || exit 1 +$exa $testcases/hiddens -l -aa 2>&1 | diff -q - $results/hiddens_laa || exit 1 + + # And finally... $exa --help | diff -q - $results/help || exit 1 $exa --help --long | diff -q - $results/help_long || exit 1 + echo "All the tests passed!"