Skip to content

Commit

Permalink
Reduce log verbosity by default.
Browse files Browse the repository at this point in the history
Fixes #141
  • Loading branch information
plietar committed Jan 5, 2017
1 parent d0a84d7 commit 7ba3d76
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
extern crate bit_set;
extern crate byteorder;
extern crate crypto;
extern crate env_logger;
extern crate eventual;
extern crate getopts;
extern crate hyper;
Expand Down
17 changes: 5 additions & 12 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
extern crate getopts;
extern crate librespot;
extern crate env_logger;
#[macro_use]
extern crate log;
extern crate ctrlc;

use std::io::{stderr, Write};
use std::process::exit;
use std::thread;
use std::env;

use librespot::spirc::SpircManager;
use librespot::main_helper;
Expand All @@ -18,27 +15,23 @@ fn usage(program: &str, opts: &getopts::Options) -> String {
}

fn main() {
if env::var("RUST_LOG").is_err() {
env::set_var("RUST_LOG", "mdns=info,librespot=trace")
}
env_logger::init().unwrap();

let mut opts = getopts::Options::new();
main_helper::add_session_arguments(&mut opts);
main_helper::add_authentication_arguments(&mut opts);
main_helper::add_player_arguments(&mut opts);
main_helper::add_program_arguments(&mut opts);

let args: Vec<String> = std::env::args().collect();

let matches = match opts.parse(&args[1..]) {
Ok(m) => m,
Err(f) => {
error!("Error: {}\n{}", f.to_string(), usage(&args[0], &opts));
exit(1)
writeln!(stderr(), "error: {}\n{}", f.to_string(), usage(&args[0], &opts)).unwrap();
exit(1);
}
};

main_helper::setup_logging(&matches);

let session = main_helper::create_session(&matches);
let credentials = main_helper::get_credentials(&session, &matches);
session.login(credentials).unwrap();
Expand Down
40 changes: 32 additions & 8 deletions src/main_helper.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use env_logger::LogBuilder;
use getopts;
use rpassword;
use std::env;
use std::io::{stderr, Write};
use std::path::PathBuf;
use std::process::exit;
Expand Down Expand Up @@ -37,7 +39,10 @@ pub fn find_backend(name: Option<&str>) -> &'static (Fn(Option<&str>) -> Box<Sin
pub fn add_session_arguments(opts: &mut getopts::Options) {
opts.optopt("c", "cache", "Path to a directory where files will be cached.", "CACHE")
.reqopt("n", "name", "Device name", "NAME")
.optopt("b", "bitrate", "Bitrate (96, 160 or 320). Defaults to 160", "BITRATE");
.optopt("b", "bitrate", "Bitrate (96, 160 or 320). Defaults to 160", "BITRATE")
.optopt("", "onstart", "Run PROGRAM when playback is about to begin.", "PROGRAM")
.optopt("", "onstop", "Run PROGRAM when playback has ended.", "PROGRAM")
.optflag("v", "verbose", "Enable verbose output");
}

pub fn add_authentication_arguments(opts: &mut getopts::Options) {
Expand All @@ -47,13 +52,8 @@ pub fn add_authentication_arguments(opts: &mut getopts::Options) {
}

pub fn add_player_arguments(opts: &mut getopts::Options) {
opts.optopt("", "backend", "Audio backend to use. Use '?' to list options", "BACKEND");
opts.optopt("", "device", "Audio device to use. Use '?' to list options", "DEVICE");
}

pub fn add_program_arguments(opts: &mut getopts::Options) {
opts.optopt("", "onstart", "Run PROGRAM when playback is about to begin.", "PROGRAM");
opts.optopt("", "onstop", "Run PROGRAM when playback has ended.", "PROGRAM");
opts.optopt("", "backend", "Audio backend to use. Use '?' to list options", "BACKEND")
.optopt("", "device", "Audio device to use. Use '?' to list options", "DEVICE");
}

pub fn create_session(matches: &getopts::Matches) -> Session {
Expand Down Expand Up @@ -136,3 +136,27 @@ pub fn create_player(session: &Session, matches: &getopts::Matches) -> Player {
make_backend(device_name.as_ref().map(AsRef::as_ref))
})
}

pub fn setup_logging(matches: &getopts::Matches) {
let verbose = matches.opt_present("verbose");
let mut builder = LogBuilder::new();

match env::var("RUST_LOG") {
Ok(config) => {
builder.parse(&config);
builder.init().unwrap();

if verbose {
warn!("`--verbose` flag overidden by `RUST_LOG` environment variable");
}
}
Err(_) => {
if verbose {
builder.parse("mdns=info,librespot=trace");
} else {
builder.parse("mdns=info,librespot=info");
}
builder.init().unwrap();
}
}
}
4 changes: 3 additions & 1 deletion src/player.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,12 @@ fn find_available_alternative<'a>(session: &Session, track: &'a Track) -> Option
fn load_track(session: &Session, track_id: SpotifyId) -> Option<vorbis::Decoder<Subfile<AudioDecrypt<Box<ReadSeek>>>>> {
let track = session.metadata::<Track>(track_id).await().unwrap();

info!("Loading track {:?}", track.name);

let track = match find_available_alternative(session, &track) {
Some(track) => track,
None => {
warn!("Track \"{}\" is not available", track.name);
warn!("Track {:?} is not available", track.name);
return None;
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl Channel {
packet.read_u16::<BigEndian>().unwrap(); // Skip channel id

if cmd == 0xa {
error!("error: {} {}", data.len(), packet.read_u16::<BigEndian>().unwrap());
trace!("error: {} {}", data.len(), packet.read_u16::<BigEndian>().unwrap());
return match handler.box_on_error(session) {
Response::Continue(_) => Response::Close,
Response::Spawn(f) => Response::Spawn(f),
Expand Down

3 comments on commit 7ba3d76

@sashahilton00
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plietar This commit has also reduced the log level of debug builds to info. Whilst one can get around it with RUST_LOG, might I suggest that the debug build ships with the log level set to trace by default?

@plietar
Copy link
Owner Author

@plietar plietar commented on 7ba3d76 Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sashahilton00 The --verbose flag can be used as well.
I'm generally opposed to having differences in behaviour between debug and release builds.

@sashahilton00
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, if you want to keep the behaviour consistent, though I would naturally assume that someone running a debug anything would want to see the logs.

Please sign in to comment.