vrruiz / xisfits

Convert images from XISF to FITS
MIT License
8 stars 0 forks source link

Use structopt for CLI #5

Closed Razican closed 5 years ago

Razican commented 5 years ago

structopt is a crate that allows generating command line interfaces that map to a struct. This would avoid this code in the main.rs file, and improve the validation:

    // Read command line arguments
    let args: Vec<String> = env::args().collect();
    if args.len() < 3 {
        eprintln!("xisfits [input file name].xifs [output file name].fits");
        process::exit(1);
    }
    println!("Args: {:?}", args);

    // Open XISF image file
    let xisf_filename = &args[1];
    let fits_filename = &args[2];

This could also be an opportunity to add a --verbose or -v mode, where all the println!() happens (Unix commands are supposed to not print anything if things go well). This would also help with the performance: writing in the terminal takes a long time.

The lazy_static dependency would probably need to be added to be able to check the CLI configuration easily from multiple sites, since it allows a 1-time initialized singleton pattern.

Let me know if this is OK with you, and I can start to work on it.

vrruiz commented 5 years ago

I agree. As features are added, using a proper option parser will become important.

vrruiz commented 5 years ago

I'm going to merge this, but ideally xisfreader.rs and fitswriter.rs shouldn't depend on CLI to enable the verbose mode. The idea is to use them like libraries.

Razican commented 5 years ago

That makes sense. Maybe they should be libraries then? We could do a small workspace to have 1/2 libraries and one executable

vrruiz commented 5 years ago

I didn't know what workspaces are. Now I do ;) Yeah, that solution is interesting.

Razican commented 5 years ago

In that regard, maybe it makes sense to change the println!() code to the log crate, and use env_logger to decide what to print.

vrruiz commented 5 years ago

I have partially added log support in xisfreader, to be able to execute tests without calling to CLI.verbose().

vrruiz commented 5 years ago

For debug messages, I removed CLI.verbose() and used env_logger instead.