Merge pull request #15 from lukasdietrich/feat/sanity-check

Sanity check for activity start- and endtimes

README.md

@@ -274,4 +274,5 @@ bartib edit   # open the activity log in the editor you have defined in your `ED
 bartib edit -e vim    # open the activity log in a given editor
 
 bartib check    # check your activity log for invalid lines
+bartib sanity    # check for activities with logical errors (e.g activities with negative duration)
 ```

src/controller/list.rs

@@ -1,6 +1,9 @@
 use anyhow::Result;
+use chrono::NaiveDateTime;
 
+use crate::conf;
 use crate::data::activity;
+use crate::data::activity::Activity;
 use crate::data::bartib_file;
 use crate::data::getter;
 use crate::view::list;
@@ -18,7 +21,11 @@ pub fn list_running(file_name: &str) -> Result<()> {
 // lists tracked activities
 //
 // the activities will be ordered chronologically.
-pub fn list(file_name: &str, filter: getter::ActivityFilter, do_group_activities: bool) -> Result<()> {
+pub fn list(
+    file_name: &str,
+    filter: getter::ActivityFilter,
+    do_group_activities: bool,
+) -> Result<()> {
     let file_content = bartib_file::get_file_content(file_name)?;
     let activities = getter::get_activities(&file_content);
     let mut filtered_activities: Vec<&activity::Activity> =
@@ -26,28 +33,98 @@ pub fn list(file_name: &str, filter: getter::ActivityFilter, do_group_activities
 
     filtered_activities.sort_by_key(|activity| activity.start);
 
-    let first_element = filtered_activities.len().saturating_sub(filter.number_of_activities.unwrap_or(filtered_activities.len()));
+    let first_element = filtered_activities.len().saturating_sub(
+        filter
+            .number_of_activities
+            .unwrap_or(filtered_activities.len()),
+    );
 
     if do_group_activities {
-        list::list_activities_grouped_by_date(
-            &filtered_activities[first_element..],
-        );
+        list::list_activities_grouped_by_date(&filtered_activities[first_element..]);
     } else {
         let with_start_dates = filter.date.is_none();
-        list::list_activities(
-            &filtered_activities[first_element..],
-            with_start_dates,
-        );
+        list::list_activities(&filtered_activities[first_element..], with_start_dates);
     }
 
     Ok(())
 }
 
+// checks the file content for sanity
+pub fn sanity_check(file_name: &str) -> Result<()> {
+    let file_content = bartib_file::get_file_content(file_name)?;
+    let mut lines_with_activities : Vec<(Option<usize>, Activity)> = file_content
+        .into_iter()
+        .filter_map(|line| {
+            match line.activity {
+                Ok(a) => Some((line.line_number, a)),
+                Err(_) => None
+            }
+        })
+        .collect();
+    lines_with_activities.sort_unstable_by_key(|(_, activity)| activity.start);
+
+    let mut has_finding : bool = false;
+    let mut last_end : Option<NaiveDateTime> = None;
+
+    for (line_number, activity) in lines_with_activities {
+        has_finding = !check_sanity(last_end, &activity, line_number) || has_finding;
+
+        if let Some(e) = last_end {
+            if let Some(this_end) = activity.end {
+                if this_end > e {
+                    last_end = Some(this_end);
+                }
+            }
+        } else {
+            last_end = activity.end;
+        }
+    }
+
+    if !has_finding {
+        println!("No unusual activities.");
+    }
+
+    Ok(())
+}
+
+fn check_sanity(last_end: Option<NaiveDateTime>, activity: &Activity, line_number: Option<usize>) -> bool {
+    let mut sane = true;
+    if activity.get_duration().num_milliseconds() < 0 {
+        println!("Activity has negative duration");
+        sane = false;
+    }
+
+    if let Some(e) = last_end {
+        if e > activity.start {
+            println!("Activity startet before another activity ended");
+            sane = false;
+        }
+    }
+
+    if !sane {
+        print_activity_with_line(activity, line_number.unwrap_or(0));
+    }
+
+    sane
+}
+
+fn print_activity_with_line(activity: &Activity, line_number: usize) {
+    println!("{} (Started: {}, Ended: {}, Line: {})\n",
+             activity.description,
+             activity.start.format(conf::FORMAT_DATETIME),
+             activity.end
+                 .map(|end| end.format(conf::FORMAT_DATETIME).to_string())
+                 .unwrap_or_else(|| String::from("--")),
+             line_number
+    )
+}
+
 // prints all errors that occured when reading the bartib file
 pub fn check(file_name: &str) -> Result<()> {
     let file_content = bartib_file::get_file_content(file_name)?;
 
-    let number_of_errors = file_content.iter()
+    let number_of_errors = file_content
+        .iter()
         .filter(|line| line.activity.is_err())
         .count();
 
@@ -58,11 +135,17 @@ pub fn check(file_name: &str) -> Result<()> {
 
     println!("Found {} line(s) with parsing errors", number_of_errors);
 
-    file_content.iter()
+    file_content
+        .iter()
         .filter(|line| line.activity.is_err() && line.plaintext.is_some())
         .for_each(|line| {
             if let Err(e) = &line.activity {
-                println!("\n{}\n  -> {} (Line: {})", line.plaintext.as_ref().unwrap(), e.to_string(), line.line_number.unwrap_or(0));
+                println!(
+                    "\n{}\n  -> {} (Line: {})",
+                    line.plaintext.as_ref().unwrap(),
+                    e,
+                    line.line_number.unwrap_or(0)
+                );
             }
         });
 
@@ -92,10 +175,11 @@ pub fn list_projects(file_name: &str, current: bool) -> Result<()> {
 pub fn list_last_activities(file_name: &str, number: usize) -> Result<()> {
     let file_content = bartib_file::get_file_content(file_name)?;
 
-    let descriptions_and_projects : Vec<(&String, &String)> = getter::get_descriptions_and_projects(&file_content);
+    let descriptions_and_projects: Vec<(&String, &String)> =
+        getter::get_descriptions_and_projects(&file_content);
     let first_element = descriptions_and_projects.len().saturating_sub(number);
 
     list::list_descriptions_and_projects(&descriptions_and_projects[first_element..]);
 
     Ok(())
-}
+}

src/data/activity.rs

@@ -19,7 +19,7 @@ pub enum ActivityError {
     #[error("could not parse date or time of activity")]
     DateTimeParseError,
     #[error("could not parse activity")]
-    GeneralParseError,
+    GeneralParseError
 }
 
 impl Activity {
@@ -76,7 +76,7 @@ impl fmt::Display for Activity {
 
 // escapes the pipe character, so we can use it to separate the distinct parts of a activity
 fn escape_special_chars(s: &str) -> String {
-    s.replace("\\", "\\\\").replace("|", "\\|")
+    s.replace('\\', "\\\\").replace('|', "\\|")
 }
 
 impl FromStr for Activity {
@@ -91,23 +91,12 @@ impl FromStr for Activity {
 
         let time_parts: Vec<&str> = parts[0].split(" - ").collect();
 
-        let starttime =
-            match NaiveDateTime::parse_from_str(time_parts[0].trim(), conf::FORMAT_DATETIME) {
-                Ok(t) => t,
-                Err(_) => return Err(ActivityError::DateTimeParseError),
-            };
-
-        let endtime: Option<NaiveDateTime>;
-
-        if time_parts.len() > 1 {
-            endtime =
-                match NaiveDateTime::parse_from_str(time_parts[1].trim(), conf::FORMAT_DATETIME) {
-                    Ok(t) => Some(t),
-                    Err(_) => return Err(ActivityError::DateTimeParseError),
-                }
+        let starttime = parse_timepart(time_parts[0])?;
+        let endtime: Option<NaiveDateTime> = if time_parts.len() > 1 {
+            Some(parse_timepart(time_parts[1])?)
         } else {
-            endtime = None;
-        }
+            None
+        };
 
         let project = parts[1].trim();
         let description = if parts.len() > 2 { parts[2].trim() } else { "" };
@@ -123,6 +112,11 @@ impl FromStr for Activity {
     }
 }
 
+fn parse_timepart(time_part: &str) -> Result<NaiveDateTime, ActivityError> {
+    NaiveDateTime::parse_from_str(time_part.trim(), conf::FORMAT_DATETIME)
+        .map_err(|_| ActivityError::DateTimeParseError)
+}
+
 /**
  * an iterator for splitting strings at the pipe character "|"
  *

src/data/bartib_file.rs

@@ -81,7 +81,7 @@ pub fn write_to_file(file_name: &str, file_content: &[Line]) -> Result<(), io::E
                 } else {
                     write!(&file_handler, "{}", line.activity.as_ref().unwrap())?
                 }
-            },
+            }
             LineStatus::Changed => write!(&file_handler, "{}", line.activity.as_ref().unwrap())?,
         }
     }

src/data/getter.rs

@@ -1,34 +1,38 @@
-use std::collections::HashSet;
 use chrono::{naive, NaiveDate};
+use std::collections::HashSet;
 
 use crate::data::activity;
-use crate::data::bartib_file;
 use crate::data::activity::Activity;
+use crate::data::bartib_file;
 
 pub struct ActivityFilter<'a> {
     pub number_of_activities: Option<usize>,
     pub from_date: Option<NaiveDate>,
     pub to_date: Option<NaiveDate>,
     pub date: Option<NaiveDate>,
-    pub project: Option<&'a str>
+    pub project: Option<&'a str>,
 }
 
-pub fn get_descriptions_and_projects(file_content: &[bartib_file::Line]) -> Vec<(&String, &String)> {
-    let mut activities : Vec<&activity::Activity> = get_activities(file_content).collect();
+pub fn get_descriptions_and_projects(
+    file_content: &[bartib_file::Line],
+) -> Vec<(&String, &String)> {
+    let mut activities: Vec<&activity::Activity> = get_activities(file_content).collect();
     get_descriptions_and_projects_from_activities(&mut activities)
 }
 
-fn get_descriptions_and_projects_from_activities<'a>(activities: &mut [&'a Activity]) -> Vec<(&'a String, &'a String)> {
+fn get_descriptions_and_projects_from_activities<'a>(
+    activities: &mut [&'a Activity],
+) -> Vec<(&'a String, &'a String)> {
     activities.sort_by_key(|activity| activity.start);
 
     /* each activity should be placed in the list in the descending order of when it had been
-        started last. To achieve this we reverse the order of the activities before we extract the
-        set of descriptions and activities. Afterwards we also reverse the list of descriptions and
-        activities.
+       started last. To achieve this we reverse the order of the activities before we extract the
+       set of descriptions and activities. Afterwards we also reverse the list of descriptions and
+       activities.
 
-        e.g. if tasks have been started in this order: a, b, c, a, c the list of descriptions and
-            activities should have this order: b, a, c
-     */
+       e.g. if tasks have been started in this order: a, b, c, a, c the list of descriptions and
+           activities should have this order: b, a, c
+    */
     activities.reverse();
 
     let mut known_descriptions_and_projects: HashSet<(&String, &String)> = HashSet::new();
@@ -51,11 +55,21 @@ pub fn get_running_activities(file_content: &[bartib_file::Line]) -> Vec<&activi
         .collect()
 }
 
-pub fn get_activities(file_content: &[bartib_file::Line]) -> impl Iterator<Item = &activity::Activity> {
+pub fn get_activities(
+    file_content: &[bartib_file::Line],
+) -> impl Iterator<Item = &activity::Activity> {
     file_content
         .iter()
-        .map(|line| line.activity.as_ref())
-        .filter_map(|activity_result| activity_result.ok())
+        .filter_map(|line: &bartib_file::Line| match &line.activity {
+            Ok(activity) => Some(activity),
+            Err(_) => {
+                println!(
+                    "Warning: Ignoring line {}. Please see `bartib check` for further information",
+                    line.line_number.unwrap_or(0),
+                );
+                None
+            }
+        })
 }
 
 pub fn filter_activities<'a>(
@@ -74,17 +88,30 @@ pub fn filter_activities<'a>(
     }
 
     activities
-        .filter(move |activity| {activity.start.date() >= from_date && activity.start.date() <= to_date})
-        .filter(move |activity| filter.project.map(|p| activity.project == *p).unwrap_or(true))
+        .filter(move |activity| {
+            activity.start.date() >= from_date && activity.start.date() <= to_date
+        })
+        .filter(move |activity| {
+            filter
+                .project
+                .map(|p| activity.project == *p)
+                .unwrap_or(true)
+        })
 }
 
 pub fn get_last_activity_by_end(file_content: &[bartib_file::Line]) -> Option<&activity::Activity> {
     get_activities(file_content)
         .filter(|activity| activity.is_stopped())
-        .max_by_key(|activity| activity.end.unwrap_or_else(||naive::MIN_DATE.and_hms(0, 0, 0)))
+        .max_by_key(|activity| {
+            activity
+                .end
+                .unwrap_or_else(|| naive::MIN_DATE.and_hms(0, 0, 0))
+        })
 }
 
-pub fn get_last_activity_by_start(file_content: &[bartib_file::Line]) -> Option<&activity::Activity> {
+pub fn get_last_activity_by_start(
+    file_content: &[bartib_file::Line],
+) -> Option<&activity::Activity> {
     get_activities(file_content).max_by_key(|activity| activity.start)
 }
 
@@ -99,11 +126,18 @@ mod tests {
         let a3 = activity::Activity::start("p2".to_string(), "d1".to_string(), None);
         let mut activities = vec![&a1, &a2, &a3];
 
-        let descriptions_and_projects = get_descriptions_and_projects_from_activities(&mut activities);
+        let descriptions_and_projects =
+            get_descriptions_and_projects_from_activities(&mut activities);
 
         assert_eq!(descriptions_and_projects.len(), 2);
-        assert_eq!(*descriptions_and_projects.get(0).unwrap(), (&"d1".to_string(), &"p1".to_string()));
-        assert_eq!(*descriptions_and_projects.get(1).unwrap(), (&"d1".to_string(), &"p2".to_string()));
+        assert_eq!(
+            *descriptions_and_projects.get(0).unwrap(),
+            (&"d1".to_string(), &"p1".to_string())
+        );
+        assert_eq!(
+            *descriptions_and_projects.get(1).unwrap(),
+            (&"d1".to_string(), &"p2".to_string())
+        );
     }
 
     #[test]
@@ -113,10 +147,17 @@ mod tests {
         let a3 = activity::Activity::start("p1".to_string(), "d1".to_string(), None);
         let mut activities = vec![&a1, &a2, &a3];
 
-        let descriptions_and_projects = get_descriptions_and_projects_from_activities(&mut activities);
+        let descriptions_and_projects =
+            get_descriptions_and_projects_from_activities(&mut activities);
 
         assert_eq!(descriptions_and_projects.len(), 2);
-        assert_eq!(*descriptions_and_projects.get(0).unwrap(), (&"d1".to_string(), &"p2".to_string()));
-        assert_eq!(*descriptions_and_projects.get(1).unwrap(), (&"d1".to_string(), &"p1".to_string()));
+        assert_eq!(
+            *descriptions_and_projects.get(0).unwrap(),
+            (&"d1".to_string(), &"p2".to_string())
+        );
+        assert_eq!(
+            *descriptions_and_projects.get(1).unwrap(),
+            (&"d1".to_string(), &"p1".to_string())
+        );
     }
 }

src/main.rs

@@ -232,6 +232,7 @@ fn main() -> Result<()> {
                 ),
         )
         .subcommand(SubCommand::with_name("check").about("checks file and reports parsing errors"))
+        .subcommand(SubCommand::with_name("sanity").about("checks sanity of bartib log"))
         .get_matches();
 
     let file_name = matches.value_of("file")
@@ -292,12 +293,13 @@ fn run_subcommand(matches: &ArgMatches, file_name: &str) -> Result<()> {
             bartib::controller::manipulation::start_editor(file_name, optional_editor_command)
         }
         ("check", Some(_)) => bartib::controller::list::check(file_name),
+        ("sanity", Some(_)) => bartib::controller::list::sanity_check(file_name),
         _ => bail!("Unknown command"),
     }
 }
 
 fn create_filter_for_arguments<'a>(sub_m: &'a ArgMatches) -> ActivityFilter<'a> {
-    let mut filter = bartib::data::getter::ActivityFilter {
+    let mut filter = ActivityFilter {
         number_of_activities: get_number_argument_or_ignore(
             sub_m.value_of("number"),
             "-n/--number",

src/view/format_util.rs

@@ -4,7 +4,7 @@ pub fn format_duration(duration: &Duration) -> String {
     let mut duration_string = String::new();
 
     if duration.num_hours() > 0 {
-        duration_string.push_str(&format!("{}h ", duration.num_hours().to_string()));
+        duration_string.push_str(&format!("{}h ", duration.num_hours()));
     }
 
     if duration.num_minutes() > 0 {

src/view/report.rs

@@ -157,9 +157,7 @@ fn group_activities_by_description<'a>(activities: &'a [&'a activity::Activity])
 
 fn get_longest_line(project_map: &ProjectMap) -> Option<usize> {
     let longest_project_line = project_map.keys().map(|p| p.chars().count()).max();
-    let longest_activity_line = project_map.values()
-        .map(|(a, _d)| a)
-        .flatten()
+    let longest_activity_line = project_map.values().flat_map(|(a, _d)| a)
         .map(|a| a.description.chars().count() + conf::REPORT_INDENTATION).max();
     get_max_option(longest_project_line, longest_activity_line)
 }
@@ -169,9 +167,7 @@ fn get_longest_duration_string(report: &Report) -> Option<usize> {
         .map(|(_a, d)| format_util::format_duration(d))
         .map(|s| s.chars().count())
         .max();
-    let longest_activity_duration = report.project_map.values()
-        .map(|(a, _d)| a)
-        .flatten()
+    let longest_activity_duration = report.project_map.values().flat_map(|(a, _d)| a)
         .map(|a| format_util::format_duration(&a.get_duration()))
         .map(|s| s.chars().count())
         .max();