Neofetch for your source code

Hi :wave:,

This is my second Rust program, I would love some feedback :grinning:

https://github.com/o2sh/onefetch

Thanks!

5 Likes

Cool little tool - the ASCII language logos are extremely cute :smile:

The main tip I'd give you is to get into the habit of running Clippy on your codebase if you're not already - it's a really helpful tool for spotting common issues in Rust code.

A few things that it picked up when I ran it on your code (all of which are fairly minor):

  • If a struct field has the same name as the variable you're using to initialize it, you can use a shorthand - e.g Struct { field: field } can become Struct { field }. This will feel familiar if you've used JavaScript in recent years!
  • self.authors.len() > 0 can be more clearly expressed as !self.authors.is_empty().
  • There's a couple of instances where you call a function in the arguments of unwrap_or - this is ineffecient, as the function will get run even if the unwrap is successful. Instead, use unwrap_or_else, which takes a closure as an argument, allowing you to hold off on constructing the default until it's actually needed!
  • If you're matching on something but only care about one possible pattern, you can use the shorter if let syntax.
  • url.split('/') (with a char literal) is slightly more efficient than url.split("/") (with a &str literal).

The only other thing that sprung out to me is that I think your implementation of Display for Info could get away with a few less String allocations by using write!/writeln! to write directly into your buffers:

impl fmt::Display for Info {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        let mut buffer = String::new();
        let color = get_color(&self.language);

        writeln!(
            buffer,
            "{}{}",
            "Project: ".color(color).bold(),
            self.project_name
        )?;
        writeln!(
            buffer,
            "{}{}",
            "Language: ".color(color).bold(),
            self.language
        )?;

        if !self.authors.is_empty() {
            let title = if self.authors.len() > 1 {
                "Authors: "
            } else {
                "Author: "
            };

            writeln!(
                buffer,
                "{}{}",
                title.color(color).bold(),
                self.authors.first().unwrap()
            )?;

            let title = " ".repeat(title.len());

            for author in self.authors.iter().skip(1) {
                writeln!(buffer, "{}{}", title.color(color).bold(), author)?;
            }
        }

        writeln!(buffer, "{}{}", "Repo: ".color(color).bold(), self.repo)?;
        writeln!(
            buffer,
            "{}{}",
            "Number of lines: ".color(color).bold(),
            self.number_of_lines
        )?;
        writeln!(
            buffer,
            "{}{}",
            "License: ".color(color).bold(),
            self.license
        )?;

        let logo = self.get_ascii();
        let mut lines = buffer.lines();
        let left_pad = logo.lines().map(|l| l.len()).max().unwrap_or(0);

        for a in logo.lines() {
            let b = match lines.next() {
                Some(line) => line,
                None => "",
            };

            writeln!(
                f,
                "{:width$} {}",
                a.color(color).bold(),
                b,
                width = left_pad
            )?;
        }

        Ok(())
    }
}

I have no idea if this is more performant, but it shouldn't make things any worse, and it's a little cleaner to my eye!

If you wanted to go optimization crazy, you could probably interleave the writing of the logo and the data in order to avoid allocating altogether, but it'd probably not worth the added complexity :stuck_out_tongue:

Anyway, there goes half an hour of my life, hope this helped!

3 Likes

Done.

Great feedback, thanks a lot for taking the time :+1:

2 Likes

Multicolor Ascii FTW :partying_face:

2 Likes