feat: Allow activities without start times #7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change introduces the ability to log activities that only have a date and a duration, without a specific start time. These are useful for accounting for work done at unspecified times during a day. Modifications include: - Enhanced date parsing to recognize 'YYYY-MM-DD' as a header for timeless activities, distinct from 'YYYY-MM-DD HH:MM' timed entries. - Updated parsing logic (`tokens_from_timed_lpr`, `parse_activities`) to correctly assign a nominal start time (midnight of the header date) to timeless activities and ensure their durations are included in summaries without interfering with the time calculations of adjacent timed activities. - Added new tests to cover various scenarios of timed and timeless activity logging. - Updated example input file to demonstrate the new format.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for logging activities without specific start times by treating date-only lines as “timeless” headers (assigned midnight) and ensuring their durations don’t shift adjacent timed entries.
- parse_date now returns an
(Timestamp, bool)to flag date-only inputs. - process_line, parse_activities, and tokens_from_timed_lpr are updated to handle timeless blocks and to only advance the clock for timed activities.
- New tests and example entries demonstrate and verify mixed timed and timeless behavior.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/parse.rs | Enhanced date parsing, updated processing flow, and tokenization |
| example/activities.txt | Added sample timeless and mixed activity entries |
| // TODO: shouldn't unwrap | ||
| Date { date } => list_of_pr.push(Date { date }), | ||
| lp => { | ||
| // Date { date } => list_of_pr.push(Date { date }), |
There was a problem hiding this comment.
Remove the commented legacy Date { date } code in parse_all_lines to reduce clutter and potential confusion.
Suggested change
| // Date { date } => list_of_pr.push(Date { date }), |
Comment on lines
+477
to
+502
| let list_of_pr_initial = parse_all_lines(&mut it); | ||
|
|
||
| let start_time = if let Some(Date { date }) = list_of_pr.first() { | ||
| *date | ||
| } else { | ||
| assert!(list_of_pr.is_empty()); | ||
| if list_of_pr_initial.is_empty() { | ||
| return vec![]; | ||
| }; | ||
|
|
||
| let mut parse_state = register_all_categories(&list_of_pr); | ||
| } | ||
|
|
||
| // Process categories based on initial parse results | ||
| let mut parse_state = register_all_categories(&list_of_pr_initial); | ||
| update_parse_state_from_config(config, &mut parse_state).expect(""); | ||
| let list_of_pr = parse_state.update_category_quadrants(list_of_pr_initial); | ||
|
|
||
| // After potential modifications by update_category_quadrants, | ||
| // it's theoretically possible list_of_pr is empty if all items were somehow removed. | ||
| // Though current logic of update_category_quadrants doesn't remove items. | ||
| if list_of_pr.is_empty() { | ||
| return vec![]; | ||
| } | ||
|
|
||
| let list_of_pr = parse_state.update_category_quadrants(list_of_pr); | ||
| // The first element of list_of_pr (which is non-empty here) must be a Date variant, | ||
| // as ensured by parse_all_lines. This date is the effective start_time for tokenization. | ||
| let first_date_in_lpr = if let Some(Date { date, .. }) = list_of_pr.first() { | ||
| *date | ||
| } else { | ||
| // This path should not be taken given the guarantees from parse_all_lines | ||
| // and the non-empty check for list_of_pr. | ||
| unreachable!("list_of_pr is non-empty and should start with a Date."); |
There was a problem hiding this comment.
[nitpick] Consider renaming list_of_pr_initial and the later list_of_pr to more descriptive names (e.g., parsed_results_initial and parsed_results) for clarity.
Comment on lines
+597
to
+598
| let datetime_formats = vec!["%Y-%m-%d %H:%M", "%Y-%m-%d %Hh%M"]; | ||
| for fmt in datetime_formats { |
There was a problem hiding this comment.
[nitpick] Extract the date-time format strings into a static array or constant to avoid reallocating a Vec each time parse_date is called.
Suggested change
| let datetime_formats = vec!["%Y-%m-%d %H:%M", "%Y-%m-%d %Hh%M"]; | |
| for fmt in datetime_formats { | |
| for fmt in DATETIME_FORMATS { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change introduces the ability to log activities that only have a date and a duration, without a specific start time. These are useful for accounting for work done at unspecified times during a day.
Modifications include:
tokens_from_timed_lpr,parse_activities) to correctly assign a nominal start time (midnight of the header date) to timeless activities and ensure their durations are included in summaries without interfering with the time calculations of adjacent timed activities.