Browse code
Changes to add better error-handling
- Relies on a modified version of smug that adds a restart to handle
errors. (http://github.com/fiddlerwoaroof/smug)
- Handles whitespace errors and invalid times.
- Allows either tabs or spaces to be used (a line can either begin with
one tab or with three spaces.)
- Version bump
Showing 5 changed files
... | ... |
@@ -8,17 +8,32 @@ The file-format is this: |
8 | 8 |
Client Name: This is a memo. |
9 | 9 |
``` |
10 | 10 |
|
11 |
-Right now the parser is fairly fragile: it reads what it can and fails silently at the first |
|
12 |
-error. Eventually there'll be better error-handling. |
|
11 |
+The parser currently has some rudimentary error handling: it detects invalid indentation (i.e. the lines of |
|
12 |
+a record do not begin with either a single tab or three spaces) and it detects invalid times. |
|
13 |
+ |
|
14 |
+Additionally, if run in interactive mode `-i`, when it discovers invalid input, it will prompt for a replacement |
|
15 |
+and attempt to correct the error. Also, with `-W`, it should be able to recover from whitespace errors on its own. |
|
16 |
+ |
|
17 |
+## Todo: |
|
18 |
+ |
|
19 |
+- Expand error handling |
|
20 |
+- Add alternative export formats |
|
21 |
+- Add querying capabilities |
|
22 |
+- Support some notion of sub-tasks |
|
23 |
+ |
|
24 |
+## Examples: |
|
13 | 25 |
|
14 | 26 |
``` |
15 | 27 |
% ./timesheet -h |
16 |
-timesheet, common-lisp version 0:1 |
|
17 |
- -c --client boolean Sort by client |
|
18 |
- -r --reverse boolean Reverse sort |
|
19 |
- -s --status boolean Print a summary of the hours worked and the prices |
|
20 |
- -h --help boolean show help |
|
21 |
-``` |
|
28 |
+timesheet file parser, version 0:4 |
|
29 |
+-c --client boolean Sort by client |
|
30 |
+-r --reverse boolean Reverse sort |
|
31 |
+-W --ignore-whitespace boolean Ignore whitespace errors in input |
|
32 |
+-i --interactive boolean Run Interactively |
|
33 |
+-v --version boolean Version |
|
34 |
+-s --status boolean Print a summary of the hours worked and the prices |
|
35 |
+-h --help boolean show help |
|
36 |
+`` |
|
22 | 37 |
|
23 | 38 |
By default, it orders the log by dates. With the `-r` option, it displays the dates in descending order: |
24 | 39 |
|
... | ... |
@@ -51,15 +51,16 @@ |
51 | 51 |
(string-join |
52 | 52 |
(list (symbol-name super) |
53 | 53 |
"-schema")))))) |
54 |
- `(prog1 |
|
55 |
- (defclass ,name ,supers |
|
56 |
- ,(list* |
|
57 |
- '(registry :initform (make-hash-table :test 'equal) :allocation :class) |
|
58 |
- (loop for element in elements |
|
59 |
- collect `(,element :initarg ,(make-keyword element) :initform nil)))) |
|
60 |
- (defclass ,schema-name ,schema-supers ()) |
|
61 |
- (defmethod slots-for append ((cls ,schema-name)) |
|
62 |
- ',elements)))) |
|
54 |
+ `(eval-when (:compile-toplevel :load-toplevel :execute) |
|
55 |
+ (prog1 |
|
56 |
+ (defclass ,name ,supers |
|
57 |
+ ,(list* |
|
58 |
+ '(registry :initform (make-hash-table :test 'equal) :allocation :class) |
|
59 |
+ (loop for element in elements |
|
60 |
+ collect `(,element :initarg ,(make-keyword element) :initform nil)))) |
|
61 |
+ (defclass ,schema-name ,schema-supers ()) |
|
62 |
+ (defmethod slots-for append ((cls ,schema-name)) |
|
63 |
+ ',elements))))) |
|
63 | 64 |
|
64 | 65 |
(define-simple-class task () |
65 | 66 |
task_id name description billable rate) |
... | ... |
@@ -80,7 +80,14 @@ |
80 | 80 |
(setf amount amnt unit unt) |
81 | 81 |
it))) |
82 | 82 |
|
83 |
-(define-condition parsing-error (parse-error) ()) |
|
83 |
+(define-condition parsing-error (parse-error) |
|
84 |
+ ((failed-chunk :initarg :failed-chunk :reader failed-chunk))) |
|
85 |
+ |
|
86 |
+(define-condition invalid-whitespace (parsing-error) () |
|
87 |
+ (:report (lambda (condition stream) |
|
88 |
+ (format stream "~s is invalid whitespace" |
|
89 |
+ (map 'list #'char-code (failed-chunk condition)))))) |
|
90 |
+ |
|
84 | 91 |
|
85 | 92 |
(define-condition invalid-day-of-week (parsing-error) |
86 | 93 |
((day-of-week :initarg :day-of-week :reader day-of-week)) |
... | ... |
@@ -155,30 +162,66 @@ |
155 | 162 |
(defun .time-separator () |
156 | 163 |
(.char= #\:)) |
157 | 164 |
|
165 |
+; TODO: consider adding one-digit hours |
|
158 | 166 |
(defun .hour () |
159 | 167 |
(.let* ((f (.first-hour-char)) |
160 | 168 |
(s (.digit))) |
161 | 169 |
(if (or (char/= f #\2) (member s '(#\0 #\1 #\2 #\3))) ;; make sure we don't get 24 |
162 |
- (.identity (parse-integer (coerce (vector f s) 'string))) |
|
170 |
+ (.identity (coerce (vector f s) 'string)) |
|
163 | 171 |
(.fail)))) |
164 | 172 |
|
165 | 173 |
(defun .minute-or-second () |
166 | 174 |
(.let* ((f (.first-minute-char)) |
167 | 175 |
(s (.digit))) |
168 |
- (.identity (parse-integer (coerce (vector f s) 'string))))) |
|
176 |
+ (.identity (coerce (vector f s) 'string)))) |
|
169 | 177 |
|
170 | 178 |
(defun .time-range-separator () |
171 | 179 |
(.string= "--")) |
172 | 180 |
|
173 |
-(defun .time () |
|
181 |
+(define-condition invalid-time (parsing-error) () |
|
182 |
+ (:report (lambda (condition stream) |
|
183 |
+ (format stream "Not a valid time part ~s" |
|
184 |
+ (failed-chunk condition))))) |
|
185 |
+ |
|
186 |
+(defun .valid-time () |
|
174 | 187 |
(.let* ((hour (.hour)) |
175 | 188 |
(_ (.time-separator)) |
176 | 189 |
(minute (.minute-or-second)) |
177 |
- #|(_ (.time-separator))|# |
|
178 | 190 |
(second (.optional |
179 | 191 |
(.progn (.time-separator) |
180 | 192 |
(.minute-or-second))))) |
181 |
- (.identity (make-time-obj hour minute (or second 0))))) |
|
193 |
+ (.identity (list hour minute (if second second "0"))))) |
|
194 |
+ |
|
195 |
+(defun .invalid-time () |
|
196 |
+ (flet ((.non-time-character () |
|
197 |
+ (.map 'string |
|
198 |
+ (.and (.not (.or (.time-separator) |
|
199 |
+ (.char= #\newline) |
|
200 |
+ (.char= #\space))) |
|
201 |
+ (.item))))) |
|
202 |
+ (.let* ((hour (.or (.hour) |
|
203 |
+ (.non-time-character))) |
|
204 |
+ (_ (.time-separator)) |
|
205 |
+ (minute (.or (.minute-or-second) |
|
206 |
+ (.non-time-character))) |
|
207 |
+ (second (.optional |
|
208 |
+ (.progn (.time-separator) |
|
209 |
+ (.or (.minute-or-second) |
|
210 |
+ (.non-time-character)))))) |
|
211 |
+ (error 'invalid-time |
|
212 |
+ :failed-chunk (concatenate 'string |
|
213 |
+ (string hour) ":" |
|
214 |
+ (string minute) |
|
215 |
+ (if second |
|
216 |
+ (concatenate 'string ":" (string second)) |
|
217 |
+ "")))))) |
|
218 |
+ |
|
219 |
+(defun .time () |
|
220 |
+ (.let* ((time (.or (.valid-time) (.invalid-time)))) |
|
221 |
+ (.identity |
|
222 |
+ (apply #'make-time-obj |
|
223 |
+ (mapcar #'parse-integer |
|
224 |
+ time))))) |
|
182 | 225 |
|
183 | 226 |
(defun .time-unit () |
184 | 227 |
(.or |
... | ... |
@@ -243,8 +286,26 @@ |
243 | 286 |
(.identity ranges) |
244 | 287 |
(.fail))))) |
245 | 288 |
|
289 |
+(defun .whitespace-char () |
|
290 |
+ (.or (.char= #\tab) (.char= #\space))) |
|
291 |
+ |
|
292 |
+(defun .whitespace () |
|
293 |
+ (.map 'string (.whitespace-char))) |
|
294 |
+ |
|
295 |
+(defun .valid-initial-space () |
|
296 |
+ (.or (.string= (string #\tab)) |
|
297 |
+ (.string= " "))) |
|
298 |
+ |
|
299 |
+(defun .extra-whitespace () |
|
300 |
+ (.let* ((_ (.valid-initial-space)) |
|
301 |
+ (extra-space (.optional (.whitespace)))) |
|
302 |
+ (if extra-space |
|
303 |
+ (error 'invalid-whitespace :failed-chunk extra-space) |
|
304 |
+ (.fail)))) |
|
305 |
+ |
|
246 | 306 |
(defun .initial-space () |
247 |
- (.string= " ")) |
|
307 |
+ (.or (.extra-whitespace) |
|
308 |
+ (.valid-initial-space))) |
|
248 | 309 |
|
249 | 310 |
(defun .time-line-start () |
250 | 311 |
(.progn (.initial-space) |
... | ... |
@@ -395,6 +456,31 @@ |
395 | 456 |
;; we don't care about the parser's output. |
396 | 457 |
(defun cdar-equal (a b) (== (cdar a) (cdar b))) |
397 | 458 |
|
459 |
+(st:deftest initial-space () |
|
460 |
+ (st:should signal invalid-whitespace |
|
461 |
+ (smug:parse (.initial-space) " ")) |
|
462 |
+ |
|
463 |
+ (st:should signal invalid-whitespace |
|
464 |
+ (smug:parse (.initial-space) (concatenate 'string |
|
465 |
+ (string #\tab) |
|
466 |
+ " "))) |
|
467 |
+ |
|
468 |
+ (st:should signal invalid-whitespace |
|
469 |
+ (smug:parse (.initial-space) (concatenate 'string |
|
470 |
+ (string #\tab) |
|
471 |
+ (string #\tab)))) |
|
472 |
+ |
|
473 |
+ (st:should signal invalid-whitespace |
|
474 |
+ (smug:parse (.initial-space) (concatenate 'string |
|
475 |
+ (string #\tab) |
|
476 |
+ " "))) |
|
477 |
+ |
|
478 |
+ (st:should be == (string #\tab) |
|
479 |
+ (smug:parse (.initial-space) (string #\tab))) |
|
480 |
+ |
|
481 |
+ (st:should be == " " |
|
482 |
+ (smug:parse (.initial-space) " "))) |
|
483 |
+ |
|
398 | 484 |
(st:deftest memo-test () |
399 | 485 |
(st:should be == '(("asdf" . "")) |
400 | 486 |
(run (.client-name) "asdf:")) |
... | ... |
@@ -434,7 +520,7 @@ |
434 | 520 |
(st:should be == '((#\, . "")) |
435 | 521 |
(run (.range-list-separator) ",")) |
436 | 522 |
|
437 |
- (st:should be == nil |
|
523 |
+ (st:should signal invalid-time |
|
438 | 524 |
(run (.range-list) "30:00:00")) |
439 | 525 |
|
440 | 526 |
(st:should be == nil |
... | ... |
@@ -496,10 +582,19 @@ |
496 | 582 |
(run (.time-range) "00:00:00--01:00:00"))) |
497 | 583 |
|
498 | 584 |
(should-test:deftest time-test () |
585 |
+ (st:should signal invalid-time |
|
586 |
+ (run (.time) "00:0a:00")) |
|
587 |
+ |
|
588 |
+ (st:should be == '(((0 0 0) . "")) |
|
589 |
+ (handler-bind ((invalid-time |
|
590 |
+ (lambda (x) x |
|
591 |
+ (smug:replace-invalid "00:0a:00" "00:00:00")))) |
|
592 |
+ (run (.time) "00:0a:00"))) |
|
593 |
+ |
|
499 | 594 |
(st:should be == '((#\: . "")) |
500 | 595 |
(run (.time-separator) ":")) |
501 | 596 |
|
502 |
- (st:should be == nil |
|
597 |
+ (st:should signal invalid-time |
|
503 | 598 |
(run (.time) "30:00:00")) |
504 | 599 |
|
505 | 600 |
(st:should be == '(((0 0 0) . "")) |
... | ... |
@@ -533,7 +628,7 @@ |
533 | 628 |
(st:should be == nil |
534 | 629 |
(run (.minute-or-second) "aa")) |
535 | 630 |
|
536 |
- (st:should be == `((1 . "")) |
|
631 |
+ (st:should be == `(("01" . "")) |
|
537 | 632 |
(run (.minute-or-second) "01"))) |
538 | 633 |
|
539 | 634 |
|
... | ... |
@@ -563,10 +658,10 @@ |
563 | 658 |
(st:should be == nil |
564 | 659 |
(run (.hour) "aa")) |
565 | 660 |
|
566 |
- (st:should be == `((20 . "")) |
|
661 |
+ (st:should be == `(("20" . "")) |
|
567 | 662 |
(run (.prog1 (.hour) (.not (.item))) "20")) |
568 | 663 |
|
569 |
- (st:should be == `((1 . "")) |
|
664 |
+ (st:should be == `(("01" . "")) |
|
570 | 665 |
(run (.prog1 (.hour) (.not (.item))) "01"))) |
571 | 666 |
|
572 | 667 |
(should-test:deftest month-test () |
... | ... |
@@ -2,13 +2,12 @@ |
2 | 2 |
|
3 | 3 |
(in-package #:timesheet) |
4 | 4 |
|
5 |
-(ubiquitous:restore 'timesheet) |
|
6 |
- |
|
7 | 5 |
;;; "timesheet" goes here. Hacks and glory await! |
8 | 6 |
|
9 | 7 |
(defmacro maybe-list (test &optional val) |
10 |
- "If both arguments passed, when test is true, return a list containing val or, when test is false, return nil. |
|
11 |
- If one argument passed, when test names something that is not a list, return a list containing it, otherwise |
|
8 |
+ "If both arguments passed, when test is true, return a list containing val |
|
9 |
+ or, when test is false, return nil. If one argument passed, when test names |
|
10 |
+ something that is not a list, return a list containing it, otherwise |
|
12 | 11 |
return nil." |
13 | 12 |
(once-only (test) |
14 | 13 |
(let ((test (if val test `(not (listp ,test)))) |
... | ... |
@@ -23,18 +22,39 @@ |
23 | 22 |
|
24 | 23 |
(defvar *default-time-sheet-file*) |
25 | 24 |
(defvar *rate*) |
26 |
- |
|
27 |
-(defun parse-file (&optional (file *default-time-sheet-file*)) |
|
28 |
- (with-open-file (s file :direction :input) |
|
29 |
- (let ((dest (make-string (file-length s)))) |
|
30 |
- (read-sequence dest s) |
|
31 |
- (multiple-value-bind (parsed leftovers) (smug:parse (timesheet.parser::.date-records) dest) |
|
32 |
- (loop |
|
33 |
- (restart-case |
|
34 |
- (if (or (null leftovers) (string= leftovers "")) |
|
35 |
- (return parsed) |
|
36 |
- (cerror "Continue?" 'parsing-error :leftovers leftovers)))) |
|
37 |
- parsed)))) |
|
25 |
+(defparameter *interactive* nil) |
|
26 |
+ |
|
27 |
+(defun parse-file (&optional (file *default-time-sheet-file*) ignore-whitespace-errors) |
|
28 |
+ (flet ((parse-string (string) |
|
29 |
+ (handler-bind ((timesheet.parser::invalid-whitespace |
|
30 |
+ (lambda (c) c |
|
31 |
+ (let ((extra-whitespace (timesheet.parser::failed-chunk c))) |
|
32 |
+ (if (or ignore-whitespace-errors |
|
33 |
+ (when *interactive* |
|
34 |
+ (y-or-n-p "Invalid extra whitespace ~s, truncate?" extra-whitespace))) |
|
35 |
+ (smug:replace-invalid extra-whitespace "") |
|
36 |
+ (progn (format t "~&Whitespace errors~%") |
|
37 |
+ (abort)))))) |
|
38 |
+ (timesheet.parser::invalid-time |
|
39 |
+ (lambda (c) c |
|
40 |
+ (let ((time (timesheet.parser::failed-chunk c))) |
|
41 |
+ (if *interactive* |
|
42 |
+ (progn |
|
43 |
+ (format *query-io* "Invalid time ~a, replacement? " time) |
|
44 |
+ (finish-output *query-io*) |
|
45 |
+ (let ((replacement (read-line))) |
|
46 |
+ (format t "~&Replacing ~s with ~s.~%---~%" time replacement) |
|
47 |
+ (smug:replace-invalid time replacement))) |
|
48 |
+ (progn |
|
49 |
+ (format t "~&Time ~a is invalid.~%" time) |
|
50 |
+ (abort))))))) |
|
51 |
+ (smug:parse (timesheet.parser::.date-records) string)))) |
|
52 |
+ (multiple-value-bind (parsed leftovers) (parse-string (read-file-into-string file)) |
|
53 |
+ (loop |
|
54 |
+ (if (or (null leftovers) (string= leftovers "")) |
|
55 |
+ (return parsed) |
|
56 |
+ (cerror "Continue?" 'parsing-error :leftovers leftovers))) |
|
57 |
+ parsed))) |
|
38 | 58 |
|
39 | 59 |
(defun unroll-date (date-obj) |
40 | 60 |
(with-slots (year month day) date-obj |
... | ... |
@@ -99,19 +119,21 @@ |
99 | 119 |
(slot-value entry 'records) |
100 | 120 |
(mapcan #'make-entry *)))) |
101 | 121 |
|
102 |
-(defun get-log (&optional (file *default-time-sheet-file*)) |
|
103 |
- (let* ((entries (parse-file file))) |
|
122 |
+(defun get-log (&optional (file *default-time-sheet-file*) ignore-whitespace) |
|
123 |
+ (let* ((entries (parse-file file ignore-whitespace))) |
|
104 | 124 |
(mapcan #'get-entry-ranges entries))) |
105 | 125 |
|
106 | 126 |
(defparameter +pprint-log-option-spec+ |
107 | 127 |
'((("client" #\c) :type boolean :optional t :documentation "Sort by client") |
108 | 128 |
(("reverse" #\r) :type boolean :optional t :documentation "Reverse sort") |
129 |
+ (("ignore-whitespace" #\W) :type boolean :optional t :documentation "Ignore whitespace errors in input") |
|
130 |
+ (("interactive" #\i) :type boolean :optional t :documentation "Run Interactively") |
|
109 | 131 |
(("version" #\v) :type boolean :optional t :documentation "Version") |
110 | 132 |
(("status" #\s) :type boolean :optional t |
111 | 133 |
:documentation "Print a summary of the hours worked and the prices") |
112 | 134 |
(("help" #\h) :type boolean :optional t :documentation "show help"))) |
113 | 135 |
|
114 |
-(defparameter *version* "0:3") |
|
136 |
+(defparameter *version* "0:4") |
|
115 | 137 |
|
116 | 138 |
(define-message version-message (version) |
117 | 139 |
(:own-line () "timesheet file parser, version " :str)) |
... | ... |
@@ -126,8 +148,7 @@ |
126 | 148 |
(defun sort-by-date (results) |
127 | 149 |
(stable-sort results #'local-time:timestamp< |
128 | 150 |
:key (alambda (apply #'local-time:encode-timestamp |
129 |
- (append '(0 0 0 0) |
|
130 |
- (unroll-date (date it))))))) |
|
151 |
+ (list* 0 0 0 0 (unroll-date (date it))))))) |
|
131 | 152 |
|
132 | 153 |
(defun group-by-client (incompletes) |
133 | 154 |
(let ((results (make-hash-table :test 'equalp))) |
... | ... |
@@ -196,7 +217,7 @@ |
196 | 217 |
(setf list tail) ; Here we step towards the terminating condition |
197 | 218 |
(go start))))) ; Recurse |
198 | 219 |
|
199 |
-(defun pprint-log (args &key client reverse status help version) |
|
220 |
+(defun pprint-log (args &key client reverse status help version ignore-whitespace interactive) |
|
200 | 221 |
(when help |
201 | 222 |
(show-help) |
202 | 223 |
(return-from pprint-log)) |
... | ... |
@@ -211,26 +232,36 @@ |
211 | 232 |
(setf results (stable-sort results #'string-lessp :key #'client))) |
212 | 233 |
(when reverse |
213 | 234 |
(setf results (nreverse results))) |
214 |
- results)) |
|
235 |
+ results) |
|
236 |
+ (get-logs (files) |
|
237 |
+ (loop for file in (ensure-list files) |
|
238 |
+ append (get-log file ignore-whitespace)) )) |
|
215 | 239 |
|
216 | 240 |
(let ((*default-time-sheet-file* (or args *default-time-sheet-file*)) |
241 |
+ (*interactive* interactive) |
|
217 | 242 |
(*print-pretty* t)) |
218 | 243 |
(let-each (:be *) |
219 |
- (loop for file in (ensure-list *default-time-sheet-file*) |
|
220 |
- append (get-log file)) |
|
244 |
+ (get-logs *default-time-sheet-file*) |
|
221 | 245 |
(group-by-class *) |
222 | 246 |
(destructuring-bind (complete-ranges incomplete-ranges) * |
223 | 247 |
(let ((complete-results (sort-results complete-ranges client)) |
224 | 248 |
(incomplete-results (sort-results incomplete-ranges t))) |
225 | 249 |
(pprint-results complete-results incomplete-results status))))))) |
226 | 250 |
|
251 |
+(defmacro with-timesheet-configuration (() &body body) |
|
252 |
+ `(progn |
|
253 |
+ (ubiquitous:restore 'timesheet) |
|
254 |
+ (let ((*rate* (ubiquitous:defaulted-value 0 :rate)) |
|
255 |
+ (*default-time-sheet-file* |
|
256 |
+ (ubiquitous:defaulted-value #p"~/time.md" :timesheet :file))) |
|
257 |
+ ,@body))) |
|
258 |
+ |
|
227 | 259 |
(defun pprint-log-main (argv) |
228 |
- (setf *rate* (ubiquitous:defaulted-value 0 :rate) |
|
229 |
- *default-time-sheet-file* (ubiquitous:defaulted-value #p"~/time.md" :timesheet :file)) |
|
230 |
- (command-line-arguments:handle-command-line |
|
231 |
- +pprint-log-option-spec+ |
|
232 |
- 'pprint-log |
|
233 |
- :command-line (cdr argv) |
|
234 |
- :name "timesheet" |
|
235 |
- :rest-arity t)) |
|
260 |
+ (with-timesheet-configuration () |
|
261 |
+ (command-line-arguments:handle-command-line |
|
262 |
+ +pprint-log-option-spec+ |
|
263 |
+ 'pprint-log |
|
264 |
+ :command-line (cdr argv) |
|
265 |
+ :name "timesheet" |
|
266 |
+ :rest-arity t))) |
|
236 | 267 |
|