filter todos from the problems view#38
Conversation
devoncarew
commented
Aug 6, 2016
- filter todos from the problems view (and add a preferences for it)
- add a hidden flag to start the analysis server diagnostic server up on a specified port
| }, | ||
| "dart.showTodos": { | ||
| "type": "boolean", | ||
| "default": false, |
There was a problem hiding this comment.
Slightly torn about this default... It could be quite noisy if you have a lot, but it's also a nice reminder you should deal with this stuff. If we default to off I suspect most people will never know the option exists, but without a way to filter it in the Problems window maybe it'll just be more annoying than anything.
I just Google'd to see if I can find what other languages do.. Didn't find any evidence of any showing TODOs, but I did find this (watch the animated GIF) which looks quite cool (but doesn't seem like something a Dart-specific extension would be expected to do!)
https://github.com/MattiasPernhult/vscode-todo
Let's leave as is for now and hope maybe we'll get a filter or something in future. I guess the file-search is a reasonable substitute if you do want to go and fix them (which I really need to on this codebase already!).
There was a problem hiding this comment.
Let's leave as is for now
I think you're right about the discovery issue - people won't know to turn it on.
That todo extension looks nice. It does seem like you want to work with todos occasionally, and not all the time like errors or warnings.
There was a problem hiding this comment.
What about collapsing the TODOs into a single line?
You have 123 TODOs. Set dart.showTodos in settings to have them displayed here
I think having one item in the list won't be a problem and it aids discovery. If we can hook the user clicking on it (I suspect not) we could even do "click to show" and then fill them in.
There was a problem hiding this comment.
Not sure how well this'd work - we have to give Code a Uri for a set. It might not accept a null or fake filename; possibly could attribute them against pubspec or similar, but then it's starting to feel a bit hacky :(
There was a problem hiding this comment.
I guess I'm leaning towards enabling it by default, and assuming that people will see the setting if they want to disable it.
Right now it takes a restart before the setting really takes effect - it filters as the analysis errors are created, and doesn't remember them to re-report to vscode.
There was a problem hiding this comment.
I'm happy for the default to be on until we have a better option (and like you say, people more likely to look for an option if they don't like it). Not applying immediately sucks a little, but we could send an analysis.reanalyze from vscode.workspace.onDidChangeConfiguration? (though it looks like we don't get told which values changed; we'd have to always do it of stash a copy of the previous value)
There was a problem hiding this comment.
+1 to reanalyzing if that setting changes, it'll be nice to not have the UI
out of sync with the pref setting
There was a problem hiding this comment.
Ok, sounds like default=on and reanalyze on change seems best for now. I've updated #39 for handling this.
|
Merging this in because I think it's all good. I'll raise an issue about investigating options for showing TODOs. |