Closed thombruce closed 3 months ago
netlify[bot]
commented
3 months ago | Name | Link |
|---|---|
| Latest commit | 908dd64c8e67a9191af6902e81266dfd410dea10 |
| Latest deploy log | https://app.netlify.com/sites/toodles/deploys/666354bc833fd90008d1f817 |
github-actions[bot]
commented
3 months ago | Status | Category | Percentage | Covered / Total |
|---|---|---|---|
| 🟢 | Lines | 69.61% / 60% | 433 / 622 |
| 🟢 | Statements | 69.61% / 60% | 433 / 622 |
| 🟢 | Functions | 65.85% / 60% | 27 / 41 |
| 🟢 | Branches | 72.05% / 60% | 49 / 68 |
thombruce
commented
3 months ago Does not presently add the additional decorations suggested in the issue; for instance, to show a total based on price multiplied by multiplier.
Since each regex is iterated over in sequence, however, it seems redundant to identify price again just to apply the multiplier to it.
This may call for a refactor first of all and a better understanding of the order of operations involved here.
I think we leave hover messages as a future exercise. I need to better understand how VS Code extensions interpret the lines of a document - handling something like sorting first may teach me this - and possibly refactor the approach to matching items such that, well, in particular such that when it comes to prices and multipliers we're handling these at the same time. 🤔
That's a thought, huh? Rather than iterate over regexes for those two, why not produce a custom function that handles them both. This would be no more or less (in)efficient than our current approach, which is to iterate over matching tasks in sequence anyway. In the case of prices and multipliers, we remove them from the iterated items and handle them both in one custom function.
thombruce
commented
3 months ago The above said, I'm still tempted to say let's refactor first. The code could certainly use some tidying up, and the idea of handling prices separately from other iterated matching patterns suggests we might explore a sort of... plugin pattern. This would mean holding the code to handle that in a different file or files, and the current codebase being mostly a single file does not lend itself to doing that.
I say, merge the code as is now, create a refactor issue, create an issue to handle this in the future, execute in that order.
thombruce
commented
3 months ago Have made the decision to match prices only near the start of the string, after dates. This will make it easier to recognise the correct price and apply the multiplier to it. Toodles list item order is now:
[status] [priority] [completed date] [created date] [due date] [price] Description of todo [multiplier]Every part of the above, apart from description, is optional. Additionally, if a completed todo has a completed date, it must also have a created date (holdover rule from todo.txt). Due date is always optional but must come after the created date.
closes #111
By submitting this pull request, you agree to follow our Code of Conduct: https://github.com/thombruce/.github/blob/main/CODE_OF_CONDUCT.md
Internal use. Do not delete.