From 1e13bbdca66168a2502017b4822f5788e30c7ef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Cerveau?= Date: Thu, 13 Jan 2022 17:24:02 +0100 Subject: [PATCH] logger: use of simplelog crate Logs are now visible in 2 different place the treeview in the bottom and in a log file with log level. --- Cargo.lock | 60 ++++++++++++++++++++ Cargo.toml | 3 +- TODO.md | 2 + src/app.rs | 68 ++++++++++++----------- src/logger.rs | 129 +++++++++++++++++++++++++++++-------------- src/plugindialogs.rs | 4 +- src/settings.rs | 9 ++- 7 files changed, 196 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 52b3a7c..45db666 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -141,6 +141,19 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "chrono" +version = "0.4.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "670ad68c9088c2a963aaa298cb369688cf3f9465ce5e2d4ca10e6e0098a1ce73" +dependencies = [ + "libc", + "num-integer", + "num-traits", + "time", + "winapi", +] + [[package]] name = "dtoa" version = "0.4.8" @@ -483,6 +496,7 @@ dependencies = [ "once_cell", "serde", "serde_any", + "simplelog", "x11", "xml-rs 0.8.4", ] @@ -1068,6 +1082,17 @@ dependencies = [ "yaml-rust", ] +[[package]] +name = "simplelog" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1348164456f72ca0116e4538bdaabb0ddb622c7d9f16387c725af3e96d6001c" +dependencies = [ + "chrono", + "log 0.4.14", + "termcolor", +] + [[package]] name = "slab" version = "0.4.5" @@ -1158,6 +1183,15 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "af547b166dd1ea4b472165569fc456cfb6818116f854690b0ff205e636523dab" +[[package]] +name = "termcolor" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" +dependencies = [ + "winapi-util", +] + [[package]] name = "thiserror" version = "1.0.30" @@ -1178,6 +1212,17 @@ dependencies = [ "syn", ] +[[package]] +name = "time" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6db9e6914ab8b1ae1c260a4ae7a49b6c5611b40328a735b21862567685e73255" +dependencies = [ + "libc", + "wasi", + "winapi", +] + [[package]] name = "tinyvec" version = "1.5.1" @@ -1267,6 +1312,12 @@ version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5fecdca9a5291cc2b8dcf7dc02453fee791a280f3743cb0905f8822ae463b3fe" +[[package]] +name = "wasi" +version = "0.10.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f" + [[package]] name = "winapi" version = "0.3.9" @@ -1283,6 +1334,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70ec6ce85bb158151cae5e5c87f95a8e97d2c0c4b001223f33a334e3ce5de178" +dependencies = [ + "winapi", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" diff --git a/Cargo.toml b/Cargo.toml index 93d8125..5ceabe5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,4 +16,5 @@ once_cell = "1.7.2" xml-rs = "0.8.4" x11 = { version = "2.18", features = ["xlib"] } serde = "1.0" -serde_any = "0.5" \ No newline at end of file +serde_any = "0.5" +simplelog = "0.11.2" \ No newline at end of file diff --git a/TODO.md b/TODO.md index 500bcfa..5a8b54e 100644 --- a/TODO.md +++ b/TODO.md @@ -50,6 +50,8 @@ - [ ] Seek to position - [ ] Icon install - [ ] Flatpak infrastructure +- [ ] handle the caps setter +- [x] Logger in file/app all over the app ## bugs diff --git a/src/app.rs b/src/app.rs index b59e8cd..f7835e5 100644 --- a/src/app.rs +++ b/src/app.rs @@ -27,6 +27,7 @@ use gtk::{ TreeView, TreeViewColumn, Viewport, Widget, }; use gtk::{gio, gio::SimpleAction, glib, graphene}; +use log::error; use once_cell::unsync::OnceCell; use std::cell::RefCell; use std::collections::HashMap; @@ -38,7 +39,7 @@ use crate::logger; use crate::pipeline::{Pipeline, PipelineState}; use crate::plugindialogs; use crate::settings::Settings; -use crate::{GPS_DEBUG, GPS_ERROR, GPS_WARN}; +use crate::{GPS_DEBUG, GPS_ERROR, GPS_TRACE, GPS_WARN}; use crate::graphmanager::{GraphView, Node, PortDirection}; @@ -119,7 +120,7 @@ impl GPSApp { let app = match GPSApp::new(application) { Ok(app) => app, Err(err) => { - println!("Error creating application: {}", err); + error!("Error creating application: {}", err); return; } }; @@ -328,35 +329,41 @@ impl GPSApp { dialog.set_resizable(false); dialog.show(); } + fn add_column_to_treeview(&self, tree_name: &str, column_name: &str, column_n: i32) { + let treeview: TreeView = self + .builder + .object(tree_name) + .expect("Couldn't get tree_name"); + let column = TreeViewColumn::new(); + let cell = CellRendererText::new(); + column.pack_start(&cell, true); + // Association of the view's column with the model's `id` column. + column.add_attribute(&cell, "text", column_n); + column.set_title(column_name); + treeview.append_column(&column); + } fn reset_logger_list(&self, logger_list: &TreeView) { - let model = ListStore::new(&[String::static_type(), String::static_type()]); + let model = ListStore::new(&[ + String::static_type(), + String::static_type(), + String::static_type(), + ]); logger_list.set_model(Some(&model)); } fn setup_logger_list(&self) { + self.add_column_to_treeview("treeview-logger", "TIME", 0); + self.add_column_to_treeview("treeview-logger", "LEVEL", 1); + self.add_column_to_treeview("treeview-logger", "LOG", 2); let logger_list: TreeView = self .builder .object("treeview-logger") .expect("Couldn't get treeview-logger"); - let column = TreeViewColumn::new(); - let cell = CellRendererText::new(); - column.pack_start(&cell, true); - // Association of the view's column with the model's `id` column. - column.add_attribute(&cell, "text", 0); - column.set_title("LEVEL"); - logger_list.append_column(&column); - let column = TreeViewColumn::new(); - let cell = CellRendererText::new(); - column.pack_start(&cell, true); - // Association of the view's column with the model's `id` column. - column.add_attribute(&cell, "text", 1); - column.set_title("LOG"); - logger_list.append_column(&column); self.reset_logger_list(&logger_list); } - fn add_to_logger_list(&self, log_entry: String) { + fn add_to_logger_list(&self, log_entry: &str) { let logger_list: TreeView = self .builder .object("treeview-logger") @@ -365,9 +372,8 @@ impl GPSApp { let list_store = model .dynamic_cast::() .expect("Could not cast to ListStore"); - if let Some(log) = log_entry.split_once('\t') { - list_store.insert_with_values(None, &[(0, &log.0), (1, &log.1)]); - } + let log: Vec<&str> = log_entry.splitn(3, ' ').collect(); + list_store.insert_with_values(None, &[(0, &log[0]), (1, &log[1]), (2, &log[2])]); } } @@ -385,14 +391,7 @@ impl GPSApp { .builder .object("treeview-favorites") .expect("Couldn't get treeview-favorites"); - let column = TreeViewColumn::new(); - let cell = CellRendererText::new(); - - column.pack_start(&cell, true); - // Association of the view's column with the model's `id` column. - column.add_attribute(&cell, "text", 0); - column.set_title("favorites"); - favorite_list.append_column(&column); + self.add_column_to_treeview("treeview-favorites", "Element", 0); self.reset_favorite_list(&favorite_list); let app_weak = self.downgrade(); favorite_list.connect_row_activated(move |tree_view, _tree_path, _tree_column| { @@ -480,11 +479,16 @@ impl GPSApp { // Setup the logger to get messages into the TreeView let (ready_tx, ready_rx) = glib::MainContext::channel(glib::PRIORITY_DEFAULT); let app_weak = self.downgrade(); - logger::init_logger(ready_tx, logger::LogLevel::Debug); + logger::init_logger( + ready_tx, + Settings::default_log_file_path() + .to_str() + .expect("Unable to convert log file path to a string"), + ); self.setup_logger_list(); let _ = ready_rx.attach(None, move |msg: String| { let app = upgrade_weak!(app_weak, glib::Continue(false)); - app.add_to_logger_list(msg); + app.add_to_logger_list(&msg); glib::Continue(true) }); @@ -598,7 +602,7 @@ impl GPSApp { glib::clone!(@weak application => @default-return None, move |values: &[Value]| { let app = upgrade_weak!(app_weak, None); let id = values[1].get::().expect("id in args[1]"); - GPS_DEBUG!("Graph updated id={}", id); + GPS_TRACE!("Graph updated id={}", id); let _ = app .save_graph( Settings::default_graph_file_path() diff --git a/src/logger.rs b/src/logger.rs index 0f90ca3..c7a9acf 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -1,24 +1,20 @@ use glib::Sender; use gtk::glib; -use once_cell::sync::Lazy; -use once_cell::sync::OnceCell; -use std::cell::RefCell; +use log::{debug, error, info, trace, warn}; +use simplelog::*; use std::fmt; -use std::sync::{Arc, Mutex}; +use std::io; -#[derive(Default)] -struct Logger { - pub log_sender: OnceCell>>>>, - pub log_level: OnceCell, -} +use std::fs::File; #[derive(Debug, Eq, Ord, PartialEq, PartialOrd)] + pub enum LogLevel { Error, Warning, Info, - Log, Debug, + Trace, } impl fmt::Display for LogLevel { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -50,14 +46,6 @@ macro_rules! GPS_INFO ( }) ); -#[macro_export] -macro_rules! GPS_LOG ( - () => ($crate::print!("\n")); - ($($arg:tt)*) => ({ - logger::print_log(logger::LogLevel::Log, format_args!($($arg)*).to_string()); - }) -); - #[macro_export] macro_rules! GPS_DEBUG ( () => ($crate::print!("\n")); @@ -66,32 +54,89 @@ macro_rules! GPS_DEBUG ( }) ); -static LOGGER: Lazy = Lazy::new(Logger::default); +#[macro_export] +macro_rules! GPS_TRACE ( + () => ($crate::print!("\n")); + ($($arg:tt)*) => ({ + logger::print_log(logger::LogLevel::Trace, format_args!($($arg)*).to_string()); + }) +); -pub fn init_logger(sender: Sender, log_level: LogLevel) { - LOGGER - .log_sender - .set(Arc::new(Mutex::new(RefCell::new(sender)))) - .expect("init logger should be called once"); - let _ = LOGGER.log_level.set(log_level); +struct WriteAdapter { + sender: Sender, + buffer: String, +} + +impl io::Write for WriteAdapter { + // On write we forward each u8 of the buffer to the sender and return the length of the buffer + fn write(&mut self, buf: &[u8]) -> io::Result { + self.buffer + .push_str(&String::from_utf8(buf.to_vec()).unwrap()); + if self.buffer.ends_with('\n') { + self.buffer.pop(); + self.sender.send(self.buffer.clone()).unwrap(); + self.buffer = String::from(""); + } + + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} + +fn translate_to_simple_logger(log_level: LogLevel) -> LevelFilter { + match log_level { + LogLevel::Error => LevelFilter::Error, + LogLevel::Warning => LevelFilter::Warn, + LogLevel::Info => LevelFilter::Info, + LogLevel::Debug => LevelFilter::Debug, + LogLevel::Trace => LevelFilter::Trace, + } +} + +pub fn init_logger(sender: Sender, log_file: &str) { + simplelog::CombinedLogger::init(vec![ + WriteLogger::new( + translate_to_simple_logger(LogLevel::Trace), + Config::default(), + File::create(log_file).unwrap(), + ), + WriteLogger::new( + translate_to_simple_logger(LogLevel::Debug), + Config::default(), + WriteAdapter { + sender, + buffer: String::from(""), + }, + ), + TermLogger::new( + LevelFilter::Info, + Config::default(), + TerminalMode::Mixed, + ColorChoice::Auto, + ), + ]) + .unwrap(); } pub fn print_log(log_level: LogLevel, msg: String) { - if log_level - <= *LOGGER - .log_level - .get() - .expect("Logger should be initialized before calling print_log") - { - let mut sender = LOGGER - .log_sender - .get() - .expect("Logger should be initialized before calling print_log") - .lock() - .expect("guarded"); - - if let Err(e) = sender.get_mut().send(format!("{}\t{}", log_level, msg)) { - println!("Error: {} for {}", e, msg); - }; - } + match log_level { + LogLevel::Error => { + error!("{}", msg); + } + LogLevel::Warning => { + warn!("{}", msg); + } + LogLevel::Info => { + info!("{}", msg); + } + LogLevel::Debug => { + debug!("{}", msg); + } + LogLevel::Trace => { + trace!("{}", msg); + } + }; } diff --git a/src/plugindialogs.rs b/src/plugindialogs.rs index a1fd1eb..bed3aa1 100644 --- a/src/plugindialogs.rs +++ b/src/plugindialogs.rs @@ -152,7 +152,7 @@ pub fn display_plugin_properties(app: &GPSApp, element_name: &str, node_id: u32) entry.set_widget_name(&name); entry.connect_changed( glib::clone!(@weak entry, @strong update_properties => move |_| { - GPS_LOG!("{}:{}", entry.widget_name(), entry.text()); + GPS_TRACE!("property changed: {}:{}", entry.widget_name(), entry.text()); update_properties.borrow_mut().insert(entry.widget_name().to_string(), entry.text().to_string()); }), ); @@ -175,6 +175,6 @@ pub fn display_plugin_properties(app: &GPSApp, element_name: &str, node_id: u32) dialog.show(); for p in update_properties.borrow().values() { - GPS_LOG!("updated properties {}", p); + GPS_TRACE!("updated properties {}", p); } } diff --git a/src/settings.rs b/src/settings.rs index 82626c3..0d21013 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -41,7 +41,7 @@ impl Settings { path.push("settings.toml"); path } - + // Public methods pub fn default_graph_file_path() -> PathBuf { let mut path = glib::user_config_dir(); path.push(config::APP_ID); @@ -49,7 +49,12 @@ impl Settings { path } - // Public methods + pub fn default_log_file_path() -> PathBuf { + let mut path = PathBuf::new(); + path.push("gstpipelinestudio.log"); + path + } + pub fn add_favorite(favorite: &str) { let mut settings = Settings::load_settings(); settings.favorites.sort();