active filters #26
active filters #26
Conversation
server.js
Outdated
|
|
||
|
|
||
| const export_filtered_tasks = (filter, callbacks) => { | ||
| // If a filter is already passed, don't bother checking taskwarrior's |
There was a problem hiding this comment.
With the use case I had for the filter in the query, this is not correct -- both the TW filters and the given filter should be applied. However, since the UI is not using the filter query parameter, maybe it would be simpler to drop it for now. Currently it's an unused feature polluting the code, so I'd argue it's better to just get rid of it.
server.js
Outdated
| cmd.push(filter) | ||
| cmd.push(ALWAYS_FILTER) | ||
| cmd.push('export') | ||
| const tw = spawn_tw(cmd) |
There was a problem hiding this comment.
I think my previous code was more readable than spreading this over multiple lines. Well, not really, but let cmd = ['rc.json.array=on', ALWAYS_FILTER, filter || '', 'export'] would be :D But maybe that's just me, feel free to ignore.
There was a problem hiding this comment.
Remember I mentioned having an empty string as argument broke everything in a weird way :-p so that doesn't really work.
There was a problem hiding this comment.
oh :D okay, sorry :D
server.js
Outdated
| tw_filter.on('line', (line) => { | ||
| if (!filter && line) filter = line | ||
| }) | ||
| tw_filter.on('close', (line) => { |
There was a problem hiding this comment.
Ditto -- close listener does not get a line.
server.js
Outdated
| tw_context.on('line', (line) => { | ||
| if (!context && line) context = line | ||
| }) | ||
| tw_context.on('close', (line) => { |
There was a problem hiding this comment.
According to https://nodejs.org/api/readline.html#readline_event_close the close callback does not receive a line argument: "The listener function is called without passing any arguments."
There was a problem hiding this comment.
Oops :-D, I am pretty sure I thought "hum, pretty sure that's not like that" then forgot 20 seconds afterwards and copy-pasta happened.
server.js
Outdated
| if (context) | ||
| cmd.push('rc.context.' + context) | ||
| else | ||
| cmd.push('rc.report.next.filter') |
There was a problem hiding this comment.
Hm, shouldn't both apply? What is the logic for applying only the context filter if defined? When you call task on the commandline with a context set, it applies both the context filter and the report filter (next, list, whatever...).
server.js
Outdated
| }) | ||
| } | ||
|
|
||
| const spawn_tw = (opts) => { |
There was a problem hiding this comment.
It was about time to make a function for this :D Thanks for not being a pig like the original author of this code! ;-)
[creating a PR so I can comment on stuff]
This change is