Catty: A mini cat clone in Rust











up vote
1
down vote

favorite












As part of my journey in learning the Rust programming language, I decided to make a miniature cat clone (catty) in it. The following is my code, which depends on clap for argument parsing (see below). It currently only supports concatenating 1 file with possibly numbered lines (-n/--number). I tried to be as close as possible to the actual cat program for this:



#[macro_use] extern crate clap;

use std::{io, error, env, fs::read_to_string, path::PathBuf, process};

fn main() {
process::exit(
if let Err(err) = cli(env::args().collect::<Vec<_>>()) {
// CLI parsing errors
if let Some(clap_err) = err.downcast_ref::<clap::Error>() {
eprint!("{}", clap_err);
} else {
eprintln!("{}", err);
}
1
} else {
0
}
);
}

fn cli(cli_args: Vec<String>) -> Result<(), Box<error::Error>> {
let matches = clap::App::new("catty")
.version(crate_version!())
.about("A minimal clone of the linux utility cat.")
.arg(clap::Arg::with_name("FILE")
.help("The file to concatenate to standard output")
.required(true))
.arg(clap::Arg::with_name("number")
.short("n")
.long("number")
.help("Numbers all output lines"))
.get_matches_from_safe(cli_args)?;

let file_contents = get_file_contents(matches.value_of("FILE").unwrap())?;
let file_contents: Vec<&str> = file_contents.split("n").collect();

let number_lines = matches.is_present("number");

for (i, line) in file_contents.iter().enumerate() {
let formatted_line = if number_lines {
format!("{:>6} {}", i + 1, line)
} else {
line.to_string()
};

if i == file_contents.len() - 1 && line.len() > 0 {
print!("{}", formatted_line);
} else if !(i == file_contents.len() - 1 && line.len() == 0) {
println!("{}", formatted_line);
}
}

Ok(())
}

fn get_file_contents(passed_argument: &str) -> Result<String, Box<error::Error>> {
let mut resolved_path = PathBuf::from(passed_argument);

if !resolved_path.exists() || !resolved_path.is_file() {
resolved_path = PathBuf::from(env::current_dir()?);
resolved_path.push(passed_argument);

if !resolved_path.exists() || !resolved_path.is_file() {
return Err(io::Error::new(io::ErrorKind::NotFound, "The passed file is either not a file or does not exist!").into());
}
}

Ok(read_to_string(resolved_path)?)
}


My Cargo.toml is as follows:



[package]
name = "catty"
version = "0.1.0"
authors = ["My Name <my@email.com>"]

[dependencies]

[dependencies.clap]
version = "2.32"
default-features = false
features = ["suggestions"]


Here is what I want to know from this code review:




  1. Is my code idiomatic rust (i.e good error handling, not overly verbose, etc)?

  2. Is my code performant or can it be improved in some way?










share|improve this question


























    up vote
    1
    down vote

    favorite












    As part of my journey in learning the Rust programming language, I decided to make a miniature cat clone (catty) in it. The following is my code, which depends on clap for argument parsing (see below). It currently only supports concatenating 1 file with possibly numbered lines (-n/--number). I tried to be as close as possible to the actual cat program for this:



    #[macro_use] extern crate clap;

    use std::{io, error, env, fs::read_to_string, path::PathBuf, process};

    fn main() {
    process::exit(
    if let Err(err) = cli(env::args().collect::<Vec<_>>()) {
    // CLI parsing errors
    if let Some(clap_err) = err.downcast_ref::<clap::Error>() {
    eprint!("{}", clap_err);
    } else {
    eprintln!("{}", err);
    }
    1
    } else {
    0
    }
    );
    }

    fn cli(cli_args: Vec<String>) -> Result<(), Box<error::Error>> {
    let matches = clap::App::new("catty")
    .version(crate_version!())
    .about("A minimal clone of the linux utility cat.")
    .arg(clap::Arg::with_name("FILE")
    .help("The file to concatenate to standard output")
    .required(true))
    .arg(clap::Arg::with_name("number")
    .short("n")
    .long("number")
    .help("Numbers all output lines"))
    .get_matches_from_safe(cli_args)?;

    let file_contents = get_file_contents(matches.value_of("FILE").unwrap())?;
    let file_contents: Vec<&str> = file_contents.split("n").collect();

    let number_lines = matches.is_present("number");

    for (i, line) in file_contents.iter().enumerate() {
    let formatted_line = if number_lines {
    format!("{:>6} {}", i + 1, line)
    } else {
    line.to_string()
    };

    if i == file_contents.len() - 1 && line.len() > 0 {
    print!("{}", formatted_line);
    } else if !(i == file_contents.len() - 1 && line.len() == 0) {
    println!("{}", formatted_line);
    }
    }

    Ok(())
    }

    fn get_file_contents(passed_argument: &str) -> Result<String, Box<error::Error>> {
    let mut resolved_path = PathBuf::from(passed_argument);

    if !resolved_path.exists() || !resolved_path.is_file() {
    resolved_path = PathBuf::from(env::current_dir()?);
    resolved_path.push(passed_argument);

    if !resolved_path.exists() || !resolved_path.is_file() {
    return Err(io::Error::new(io::ErrorKind::NotFound, "The passed file is either not a file or does not exist!").into());
    }
    }

    Ok(read_to_string(resolved_path)?)
    }


    My Cargo.toml is as follows:



    [package]
    name = "catty"
    version = "0.1.0"
    authors = ["My Name <my@email.com>"]

    [dependencies]

    [dependencies.clap]
    version = "2.32"
    default-features = false
    features = ["suggestions"]


    Here is what I want to know from this code review:




    1. Is my code idiomatic rust (i.e good error handling, not overly verbose, etc)?

    2. Is my code performant or can it be improved in some way?










    share|improve this question
























      up vote
      1
      down vote

      favorite









      up vote
      1
      down vote

      favorite











      As part of my journey in learning the Rust programming language, I decided to make a miniature cat clone (catty) in it. The following is my code, which depends on clap for argument parsing (see below). It currently only supports concatenating 1 file with possibly numbered lines (-n/--number). I tried to be as close as possible to the actual cat program for this:



      #[macro_use] extern crate clap;

      use std::{io, error, env, fs::read_to_string, path::PathBuf, process};

      fn main() {
      process::exit(
      if let Err(err) = cli(env::args().collect::<Vec<_>>()) {
      // CLI parsing errors
      if let Some(clap_err) = err.downcast_ref::<clap::Error>() {
      eprint!("{}", clap_err);
      } else {
      eprintln!("{}", err);
      }
      1
      } else {
      0
      }
      );
      }

      fn cli(cli_args: Vec<String>) -> Result<(), Box<error::Error>> {
      let matches = clap::App::new("catty")
      .version(crate_version!())
      .about("A minimal clone of the linux utility cat.")
      .arg(clap::Arg::with_name("FILE")
      .help("The file to concatenate to standard output")
      .required(true))
      .arg(clap::Arg::with_name("number")
      .short("n")
      .long("number")
      .help("Numbers all output lines"))
      .get_matches_from_safe(cli_args)?;

      let file_contents = get_file_contents(matches.value_of("FILE").unwrap())?;
      let file_contents: Vec<&str> = file_contents.split("n").collect();

      let number_lines = matches.is_present("number");

      for (i, line) in file_contents.iter().enumerate() {
      let formatted_line = if number_lines {
      format!("{:>6} {}", i + 1, line)
      } else {
      line.to_string()
      };

      if i == file_contents.len() - 1 && line.len() > 0 {
      print!("{}", formatted_line);
      } else if !(i == file_contents.len() - 1 && line.len() == 0) {
      println!("{}", formatted_line);
      }
      }

      Ok(())
      }

      fn get_file_contents(passed_argument: &str) -> Result<String, Box<error::Error>> {
      let mut resolved_path = PathBuf::from(passed_argument);

      if !resolved_path.exists() || !resolved_path.is_file() {
      resolved_path = PathBuf::from(env::current_dir()?);
      resolved_path.push(passed_argument);

      if !resolved_path.exists() || !resolved_path.is_file() {
      return Err(io::Error::new(io::ErrorKind::NotFound, "The passed file is either not a file or does not exist!").into());
      }
      }

      Ok(read_to_string(resolved_path)?)
      }


      My Cargo.toml is as follows:



      [package]
      name = "catty"
      version = "0.1.0"
      authors = ["My Name <my@email.com>"]

      [dependencies]

      [dependencies.clap]
      version = "2.32"
      default-features = false
      features = ["suggestions"]


      Here is what I want to know from this code review:




      1. Is my code idiomatic rust (i.e good error handling, not overly verbose, etc)?

      2. Is my code performant or can it be improved in some way?










      share|improve this question













      As part of my journey in learning the Rust programming language, I decided to make a miniature cat clone (catty) in it. The following is my code, which depends on clap for argument parsing (see below). It currently only supports concatenating 1 file with possibly numbered lines (-n/--number). I tried to be as close as possible to the actual cat program for this:



      #[macro_use] extern crate clap;

      use std::{io, error, env, fs::read_to_string, path::PathBuf, process};

      fn main() {
      process::exit(
      if let Err(err) = cli(env::args().collect::<Vec<_>>()) {
      // CLI parsing errors
      if let Some(clap_err) = err.downcast_ref::<clap::Error>() {
      eprint!("{}", clap_err);
      } else {
      eprintln!("{}", err);
      }
      1
      } else {
      0
      }
      );
      }

      fn cli(cli_args: Vec<String>) -> Result<(), Box<error::Error>> {
      let matches = clap::App::new("catty")
      .version(crate_version!())
      .about("A minimal clone of the linux utility cat.")
      .arg(clap::Arg::with_name("FILE")
      .help("The file to concatenate to standard output")
      .required(true))
      .arg(clap::Arg::with_name("number")
      .short("n")
      .long("number")
      .help("Numbers all output lines"))
      .get_matches_from_safe(cli_args)?;

      let file_contents = get_file_contents(matches.value_of("FILE").unwrap())?;
      let file_contents: Vec<&str> = file_contents.split("n").collect();

      let number_lines = matches.is_present("number");

      for (i, line) in file_contents.iter().enumerate() {
      let formatted_line = if number_lines {
      format!("{:>6} {}", i + 1, line)
      } else {
      line.to_string()
      };

      if i == file_contents.len() - 1 && line.len() > 0 {
      print!("{}", formatted_line);
      } else if !(i == file_contents.len() - 1 && line.len() == 0) {
      println!("{}", formatted_line);
      }
      }

      Ok(())
      }

      fn get_file_contents(passed_argument: &str) -> Result<String, Box<error::Error>> {
      let mut resolved_path = PathBuf::from(passed_argument);

      if !resolved_path.exists() || !resolved_path.is_file() {
      resolved_path = PathBuf::from(env::current_dir()?);
      resolved_path.push(passed_argument);

      if !resolved_path.exists() || !resolved_path.is_file() {
      return Err(io::Error::new(io::ErrorKind::NotFound, "The passed file is either not a file or does not exist!").into());
      }
      }

      Ok(read_to_string(resolved_path)?)
      }


      My Cargo.toml is as follows:



      [package]
      name = "catty"
      version = "0.1.0"
      authors = ["My Name <my@email.com>"]

      [dependencies]

      [dependencies.clap]
      version = "2.32"
      default-features = false
      features = ["suggestions"]


      Here is what I want to know from this code review:




      1. Is my code idiomatic rust (i.e good error handling, not overly verbose, etc)?

      2. Is my code performant or can it be improved in some way?







      console rust






      share|improve this question













      share|improve this question











      share|improve this question




      share|improve this question










      asked Nov 25 at 23:19









      Arnav Borborah

      693120




      693120






















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          1
          down vote



          accepted










          4: use std::{env, error, fs::read_to_string, io, path::PathBuf, process};



          Just a personal taste, but I would do



          use std::{env, error, io, process};
          use std::fs::read_to_string;
          use std::path::PathBuf;


          Yes, nested includes are nice and easy, but hard to extend. It's up to you.





          35: let file_contents: Vec<&str> = file_contents.split('n').collect();



          I would suggest using lines instead.



          Also, you can omit &str or change the line completly.



          Either



          let file_contents: Vec<_> = file_contents.lines().collect();


          or



          let file_contents = file_contents.lines().collect::<Vec<_>>();




          46/48: line.len() > 0 / line.len() == 0



          Replace that by !line.is_empty() and line.is_empty().





          60: resolved_path = PathBuf::from(env::current_dir()?);



          Remove PathBuf::from completly, because current_dir is already a PathBuf






          share|improve this answer





















          • Also, is there an equivalent of enumerate for an iterator (so I can avoid collecting into a vector)?
            – Arnav Borborah
            Nov 26 at 20:31






          • 1




            Yes, you can call enumerate on every iterator. The thing is, that you don't know the length of your iterator, which is the reason why I didn't mentioned it. If you are willing to use println! every iteration, you can simplify your code/loop a lot.
            – hellow
            Nov 27 at 7:03











          Your Answer





          StackExchange.ifUsing("editor", function () {
          return StackExchange.using("mathjaxEditing", function () {
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          });
          });
          }, "mathjax-editing");

          StackExchange.ifUsing("editor", function () {
          StackExchange.using("externalEditor", function () {
          StackExchange.using("snippets", function () {
          StackExchange.snippets.init();
          });
          });
          }, "code-snippets");

          StackExchange.ready(function() {
          var channelOptions = {
          tags: "".split(" "),
          id: "196"
          };
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function() {
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled) {
          StackExchange.using("snippets", function() {
          createEditor();
          });
          }
          else {
          createEditor();
          }
          });

          function createEditor() {
          StackExchange.prepareEditor({
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: true,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          imageUploader: {
          brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
          contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
          allowUrls: true
          },
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          });


          }
          });














          draft saved

          draft discarded


















          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f208408%2fcatty-a-mini-cat-clone-in-rust%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown

























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          1
          down vote



          accepted










          4: use std::{env, error, fs::read_to_string, io, path::PathBuf, process};



          Just a personal taste, but I would do



          use std::{env, error, io, process};
          use std::fs::read_to_string;
          use std::path::PathBuf;


          Yes, nested includes are nice and easy, but hard to extend. It's up to you.





          35: let file_contents: Vec<&str> = file_contents.split('n').collect();



          I would suggest using lines instead.



          Also, you can omit &str or change the line completly.



          Either



          let file_contents: Vec<_> = file_contents.lines().collect();


          or



          let file_contents = file_contents.lines().collect::<Vec<_>>();




          46/48: line.len() > 0 / line.len() == 0



          Replace that by !line.is_empty() and line.is_empty().





          60: resolved_path = PathBuf::from(env::current_dir()?);



          Remove PathBuf::from completly, because current_dir is already a PathBuf






          share|improve this answer





















          • Also, is there an equivalent of enumerate for an iterator (so I can avoid collecting into a vector)?
            – Arnav Borborah
            Nov 26 at 20:31






          • 1




            Yes, you can call enumerate on every iterator. The thing is, that you don't know the length of your iterator, which is the reason why I didn't mentioned it. If you are willing to use println! every iteration, you can simplify your code/loop a lot.
            – hellow
            Nov 27 at 7:03















          up vote
          1
          down vote



          accepted










          4: use std::{env, error, fs::read_to_string, io, path::PathBuf, process};



          Just a personal taste, but I would do



          use std::{env, error, io, process};
          use std::fs::read_to_string;
          use std::path::PathBuf;


          Yes, nested includes are nice and easy, but hard to extend. It's up to you.





          35: let file_contents: Vec<&str> = file_contents.split('n').collect();



          I would suggest using lines instead.



          Also, you can omit &str or change the line completly.



          Either



          let file_contents: Vec<_> = file_contents.lines().collect();


          or



          let file_contents = file_contents.lines().collect::<Vec<_>>();




          46/48: line.len() > 0 / line.len() == 0



          Replace that by !line.is_empty() and line.is_empty().





          60: resolved_path = PathBuf::from(env::current_dir()?);



          Remove PathBuf::from completly, because current_dir is already a PathBuf






          share|improve this answer





















          • Also, is there an equivalent of enumerate for an iterator (so I can avoid collecting into a vector)?
            – Arnav Borborah
            Nov 26 at 20:31






          • 1




            Yes, you can call enumerate on every iterator. The thing is, that you don't know the length of your iterator, which is the reason why I didn't mentioned it. If you are willing to use println! every iteration, you can simplify your code/loop a lot.
            – hellow
            Nov 27 at 7:03













          up vote
          1
          down vote



          accepted







          up vote
          1
          down vote



          accepted






          4: use std::{env, error, fs::read_to_string, io, path::PathBuf, process};



          Just a personal taste, but I would do



          use std::{env, error, io, process};
          use std::fs::read_to_string;
          use std::path::PathBuf;


          Yes, nested includes are nice and easy, but hard to extend. It's up to you.





          35: let file_contents: Vec<&str> = file_contents.split('n').collect();



          I would suggest using lines instead.



          Also, you can omit &str or change the line completly.



          Either



          let file_contents: Vec<_> = file_contents.lines().collect();


          or



          let file_contents = file_contents.lines().collect::<Vec<_>>();




          46/48: line.len() > 0 / line.len() == 0



          Replace that by !line.is_empty() and line.is_empty().





          60: resolved_path = PathBuf::from(env::current_dir()?);



          Remove PathBuf::from completly, because current_dir is already a PathBuf






          share|improve this answer












          4: use std::{env, error, fs::read_to_string, io, path::PathBuf, process};



          Just a personal taste, but I would do



          use std::{env, error, io, process};
          use std::fs::read_to_string;
          use std::path::PathBuf;


          Yes, nested includes are nice and easy, but hard to extend. It's up to you.





          35: let file_contents: Vec<&str> = file_contents.split('n').collect();



          I would suggest using lines instead.



          Also, you can omit &str or change the line completly.



          Either



          let file_contents: Vec<_> = file_contents.lines().collect();


          or



          let file_contents = file_contents.lines().collect::<Vec<_>>();




          46/48: line.len() > 0 / line.len() == 0



          Replace that by !line.is_empty() and line.is_empty().





          60: resolved_path = PathBuf::from(env::current_dir()?);



          Remove PathBuf::from completly, because current_dir is already a PathBuf







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered Nov 26 at 17:00









          hellow

          1965




          1965












          • Also, is there an equivalent of enumerate for an iterator (so I can avoid collecting into a vector)?
            – Arnav Borborah
            Nov 26 at 20:31






          • 1




            Yes, you can call enumerate on every iterator. The thing is, that you don't know the length of your iterator, which is the reason why I didn't mentioned it. If you are willing to use println! every iteration, you can simplify your code/loop a lot.
            – hellow
            Nov 27 at 7:03


















          • Also, is there an equivalent of enumerate for an iterator (so I can avoid collecting into a vector)?
            – Arnav Borborah
            Nov 26 at 20:31






          • 1




            Yes, you can call enumerate on every iterator. The thing is, that you don't know the length of your iterator, which is the reason why I didn't mentioned it. If you are willing to use println! every iteration, you can simplify your code/loop a lot.
            – hellow
            Nov 27 at 7:03
















          Also, is there an equivalent of enumerate for an iterator (so I can avoid collecting into a vector)?
          – Arnav Borborah
          Nov 26 at 20:31




          Also, is there an equivalent of enumerate for an iterator (so I can avoid collecting into a vector)?
          – Arnav Borborah
          Nov 26 at 20:31




          1




          1




          Yes, you can call enumerate on every iterator. The thing is, that you don't know the length of your iterator, which is the reason why I didn't mentioned it. If you are willing to use println! every iteration, you can simplify your code/loop a lot.
          – hellow
          Nov 27 at 7:03




          Yes, you can call enumerate on every iterator. The thing is, that you don't know the length of your iterator, which is the reason why I didn't mentioned it. If you are willing to use println! every iteration, you can simplify your code/loop a lot.
          – hellow
          Nov 27 at 7:03


















          draft saved

          draft discarded




















































          Thanks for contributing an answer to Code Review Stack Exchange!


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          Use MathJax to format equations. MathJax reference.


          To learn more, see our tips on writing great answers.





          Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


          Please pay close attention to the following guidance:


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          To learn more, see our tips on writing great answers.




          draft saved


          draft discarded














          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f208408%2fcatty-a-mini-cat-clone-in-rust%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown





















































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown

































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown







          Popular posts from this blog

          Сан-Квентин

          8-я гвардейская общевойсковая армия

          Алькесар