Respect year directive when sorting transactions#467
Conversation
When sorting transactions, dates written without a year component (for example `07/17` under a `year 2014' directive) were not parsed by `ledger-parse-iso-date' -- it requires a four-digit year -- so they fell through to `(float-time nil)', which returns the current time. The result was that short-dated transactions sorted as if they occurred "today", regardless of their surrounding `year' context. `ledger-sort-startkey' now inspects the date at point with `ledger-iso-date-regexp' and, for bare month/day forms, consults the most recent `year NNNN' or `Y NNNN' directive (looking through any active narrowing) to compute the intended year. If no directive is in scope, the current calendar year is used -- matching Ledger's own semantics for year-less dates. Fixes ledger/ledger#1068.
| (beginning-of-line) | ||
| (insert "\n; Ledger-mode: End sort\n\n")) | ||
|
|
||
| (defconst ledger-sort--year-directive-regex |
There was a problem hiding this comment.
This should probably go in ledger-regex.el for consistency.
| "Regex matching a `year NNNN' or `Y NNNN' directive at the start of a line. | ||
| The year number is captured in group 1.") | ||
|
|
||
| (defun ledger-sort--preceding-year () |
There was a problem hiding this comment.
This algorithm is very quadratic. On my personal ledger file with 13k transactions (3 MB file), if I convert it to use year directives instead of YYYY-MM-DD, ledger-sort-buffer takes 6-7 seconds just gathering sort keys.
Perhaps a better approach would be to, before sorting, scan once from the beginning of the buffer until the end of the sort region for year directives, and look up the year for a transaction based on its position in the buffer relative to the year directives found (which ought to be a pretty small number).
| (let ((month (string-to-number (match-string 1))) | ||
| (day (string-to-number (match-string 2))) | ||
| (year (or (ledger-sort--preceding-year) | ||
| (nth 5 (decode-time))))) |
There was a problem hiding this comment.
It seems very strange for this function to depend on the actual current wall time. Is that compatible with the actual semantics of ledger here? (admittedly I do not personally use year directives)
| (cond | ||
| ((looking-at ledger-iso-date-regexp) | ||
| (ledger-parse-iso-date (match-string 0))) | ||
| ((looking-at "\\([0-9]\\{1,2\\}\\)[-/]\\([0-9]\\{1,2\\}\\)\\(?:[^-/0-9]\\|$\\)") |
There was a problem hiding this comment.
I think this is more-or-less equivalent to ledger-incomplete-date-regexp.
2 participants
Summary
ledger-sort-startkey' now parses the date at point vialedger-iso-date-regexp' and correctly handles non-padded dates (e.g. `2015/5/7').07/17') are interpreted relative to the most recentyear NNNN' or `Y NNNN' directive preceding the transaction, looking through any active narrowing so sorting a sub-region still sees the enclosing directive. The current calendar year is used when no directive is in scope.(float-time nil)', which returns "now", so they sorted as today's date regardless of the surroundingyear' context.Fixes ledger/ledger#1068.
Test plan
make testintest/passes (241 tests).test/sort-test.elcover:2015/5/7' sorts before2015/5/10').