5
\$\begingroup\$

I want to write a custom ls command to learn rust.

This is what I've done as for the version 1.0. This display files and folders in a cyan or white if it's on a TTY. Otherwise I just print them.

I'm using atty and crossterm crates.

What I want reviewed

  • Is there a better way to deal with OsStr?
  • Is there a better way to convert OsStr to print-able strings?
  • Is there a better way to handle errors?
  • Any other area is fair game too.
  • Is my enum use good?

use std::fs::{self, DirEntry};
use std::path::Path;

use atty::Stream;
use crossterm::{Color, Colored};

enum FileEntryType {
    File,
    Dir,
}


fn main() {
    // WHY: no need to print colours on non tty
    if atty::is(Stream::Stdout) {
        visit_location(Path::new("."), &color_print);
    } else {
        visit_location(Path::new("."), &normal_print);
    }
}

fn visit_location(dir: &Path, cb: &dyn Fn(FileEntryType, &DirEntry)) {
    if !dir.is_dir() {
        return;
    }

    if let Ok(entries) = fs::read_dir(dir) {
        for entry in entries {
            if let Ok(entry) = entry {
                let path = entry.path();
                if path.is_dir() {
                    cb(FileEntryType::Dir, &entry);
                } else {
                    cb(FileEntryType::File, &entry);
                }
            }
        }
    } else {
        println!("Failed to list {}", dir.as_os_str().to_str().unwrap());
    }
}

fn color_print(file_kind: FileEntryType, p: &DirEntry) {
    match file_kind {
        FileEntryType::File => println!("{}{}", Colored::Fg(Color::White), p.file_name().as_os_str().to_str().unwrap()),
        FileEntryType::Dir => println!("{}{}", Colored::Fg(Color::Cyan), p.file_name().as_os_str().to_str().unwrap())
    }
}

fn normal_print(_: FileEntryType, p: &DirEntry) {
    println!("{}", p.file_name().as_os_str().to_str().unwrap());
}

How it looks like

Screenshot of terminal output

\$\endgroup\$
4
+50
\$\begingroup\$

OsStr conversion

According to the path source, s.as_os_str().to_str() is the same as s.to_str():

pub fn as_os_str(&self) -> &OsStr {
    &self.inner
}

pub fn to_str(&self) -> Option<&str> {
    self.inner.to_str()
}

Moreover, using unwrap() here is not safe at all as invalid Unicode is pretty common (e.g. when mounting file system with alien character encoding). There is Path::to_string_lossy that converts path to a printable string.


Using crossterm

crossterm provides helper macros to output strings. It is more idiomatic to use them instead of println!("{}{}", ...): execute and queue.

Using queue may be beneficial in case of outputting to the pipe instead of stdout.


Reporting errors

It is common practice to report errors to stderr. It also may be more helpful to show error reason when fs::read_dir() fails. You can also use crossterm to make it more colorful:

match fs::read_dir(dir) {
    Err(err) =>
        execute!(
            stdout(),
            Colored::Fg(Color::Red),
            Output(format!("Failed to list {}: {}", dir.to_string_lossy(), err)),
            Reset),
    Ok(entries) => ..
}

Callbacks

Using callbacks to print colorful vs. normal is a nice idea, but is not future-proof. It will be hard to add other output options like short/long view, hidden files, etc.

\$\endgroup\$
  • \$\begingroup\$ Good catch on the crossterm macro. Very informative. \$\endgroup\$ – bhathiya-perera Oct 22 '19 at 10:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.