naggie / dstask

Git powered terminal-based todo/note manager -- markdown note page per task. Single binary!
https://calbryant.uk/blog/dstask-a-taskwarrior-alternative/
MIT License
801 stars 47 forks source link

unify building a filter that combines the ctx and cmdLine #88

Closed Dieterbe closed 3 years ago

Dieterbe commented 3 years ago

this made some more sense of the code as i was figuring out how the ctx and cmdLine is being combined in various commands. some way to centralize building the combined filter seems sensible, though i'm not sure this is the best way to do it.

Note that some commands do not construct the combined filter in the same way. In particular CommandModify and CommandShowUnorganised stand out. I'm not sure if this is a mistake or deliberate, but I left it alone for now.

Dieterbe commented 3 years ago

Note that some commands do not construct the combined filter in the same way. In particular CommandModify and CommandShowUnorganised stand out. I'm not sure if this is a mistake or deliberate, but I left it alone for now.

this needs to be looked at. ideally they would use the new helper function. but it's out of my expertise to understand why these are different. though superficially, looks like bugs.

naggie commented 3 years ago

This looks very similar to what we had from before the filtering refactor. What happened before is logically, the tasks are loaded, the context filters out tasks that don't match, then the cmdLine (aka filter) filtered the rest into the tasks that were displayed. In reality, to deal with this the context was merged into the cmdLine first to deal with any conflicts I think.

To be honest I think I prefer the old way of filtering that we had. This is a step back in the right direction though. Happy to merge, thanks both.