Problems with views

[ghost]

Embedded

"todo.embedded.exclude" is not working. On each subsequent refresh, more files from excluded directories are displayed.

Steps to reproduce:

1. Enter a workspace containing folders/files listed under todo.embedded.exclude like node_modules
2. Refresh todos multiple times to watch the tree grow

In addition to this, could "todo.embedded.view.wholeLine" trim leading and trailing whitespace or comment characters? Viewing the whole line is not very useful because it includes indentation and most people want to see this format:

TODO: This is a todo item

Instead of:

       //TODO: This is a todo item

Having "todo.embedded.view.wholeLine" : false doesn't always work because some people do:

TODO(bob): Do this ...

Which would currently result in

// TODO

and having

/* TODO */

displays

*/

File

Like the suggestion above, indentation should be trimmed from the values. This is especially disturbing with nested headers:

Todo1:
    Todo2:
         Todo3:
             ☐ do this

which would (roughly) be shown as

> Todo1:
  >      Todo2:
    >         Todo3:
                         ☐ do this

[fabiospampinato]

What version of Todo+ are you using? What OS are you using?

[fabiospampinato]

"todo.embedded.exclude" is not working. On each subsequent refresh, more files from excluded directories are displayed.

I can't reproduce this, not on macOS nor on Windows. Can you zip me a folder where you see this happening? And can you paste here your settings if you changed them?

In addition to this, could "todo.embedded.view.wholeLine" trim leading and trailing whitespace or comment characters?

I believe leading whitespace characters are trimmed automatically by VSC, I can't see them on macOS and Windows. Are you using tabs instead of spaces? (Maybe those are not trimmed? I don't know)

most people want to see this format:
TODO: This is a todo item
Instead of:
//TODO: This is a todo item

IMO, unless for some reasons you're not using icons (todo.embedded.view.icons) and you're not grouping by type (todo.embedded.view.groupByType), having This is a todo item with an icon next to it is superior to both of those other formats.

Regarding the extra whitespace: sure, that shouldn't be there, but I can't reproduce your problem.

Having "todo.embedded.view.wholeLine" : false doesn't always work because some people do:
TODO(bob): Do this ...
Which would currently result in
// TODO

That doesn't mean that it doesn't work. Just change your regex (todo.embedded.regex) accordingly. If this is a common enough use case I might add support to this to the default regex, but I don't think I've actually ever seen that in the wild before.

and having
/* TODO */
displays
*/

I think the whole line should be displayed here (/* TODO */), give that the comment has no description (/* TODO: some description */), I'll try to add support for this to the regex. It's getting quite complicated though.

Like the suggestion above, indentation should be trimmed from the values. This is especially disturbing with nested headers:

Again, I can't reproduce this. I can just add a line for trimming whitespaces, but I'd like to know why this is not happening on your system first.

[ghost]

Just visiting the installation folder of this extension I can reproduce the problem with default settings. I'm running Arch Linux with the latest release of todo+. I'm also using 4 spaces as the indentation.

[fabiospampinato]

@nealot can you check if embedded todos' whitespaces are trimmed for you in v4.1.0?

[ghost]

Yeah the whitespaces are trimmed when enabling whole lines now. Although you're probably right that it looks better without it enabled.

[fabiospampinato]

If you need to look at the code you can just hover over a todo and you'll see the code preceding it in the tooltip.

I'll tackle the other problems you had in the following days, let me know if you find any other problems.

[ghost]

Thanks for the tip about the hovers!

Here is what I see when testing out the different providers:

Using Ripgrep (after pressing refresh)

Using silver searcher

Using Javascript (prior to pressing refresh)

Using Javascript (after pressing refresh)

I tested this out in the "extension/fabiospampinato.vscode-todo-plus-4.0.6/" folder.

And for reference, here is what the file view I was mentioning looks like:

[ghost]

On a side note, it would be neat if vscode added an api for badges so the headers would look like this:

[fabiospampinato]

ripgrep doesn't support lookaheads, which are used by default in todo.embedded.regex, this is why there are no results. The "No embedded todos found" message shouldn't be rendered inside another element though, I can reproduce it so I should be able to fix it soon.

I don't know why the silver searcher is looking into the node_modules directory, I can't reproduce that. Same thing about refreshing when using the javascript provider.

I'll fix the file view on the next update, but I've still no idea why spaces are not trimmed on your system though.

Yeah an API for badges would be cool, there's an issue about that microsoft/vscode#38426.

[muuvmuuv]

@fabiospampinato what would be the default regex for ripgrep? I get a similiar problem also also a huge CPU load when using AG provider each time I change the view or add a include to the embedded.

[fabiospampinato]

@muuvmuuv there's only one regex, the one defined in todo.embedded.regex, you'll have to write one on your own if you want to use ripgrep. Unfortunately I thought about that, but I don't know if it's possible to write a regex without lookaheads that properly matches things like //TODO: foo //TODO: bar.

I think AG is multi-threaded, it tries to parallelize work as much as possible, hence the CPU spike. It should be pretty quick/lightweight on refreshes though, isn't it? Are you also seeing results from node_modules?

[muuvmuuv]

Yep, I'll work with ag, it works better with vscode.

Nope, but from other directories that are excluded:

{
  "todo.embedded.exclude": [
    "**/.git",
    "**/node_modules",
    "**/vendor",
    "**/tmp",
    "**/logs",
    "**/vendor",
    "**/bin",
    "**/cache",
    "**/cli",
    "**/libs",
    "**/dist",
    "**/assets",
    "**/administrator",
    "**/plugins"
  ],
  "todo.embedded.include": [
    "modules/mod_additive_*/**", "plugins/content/additive_*/**", "components/com_add*/**", "templates/additive/**"
  ],
}

Those administrator and plugins are still in the embedded view.

[fabiospampinato]

Include and exclude patterns aren't currently supported by ag and rg. Besides, as I mention in the readme, ag doesn't support double-star patterns (those containing **), even though it says it does.

I'll fix this in the next release on the one after that 👍

[muuvmuuv]

Ah okay, I just copied the double-stars from your docs, that's why I thought it would work. I'll wait for the next release. And javascript is the only one who can exclude and include right? But doesn't rg can also exclude files?

[fabiospampinato]

Both ag and rg can exclude stuff, they parse your .gitignore file, or, if you have one, your .ignore file. So at the moment if you want to filter something with them you'll have to put your exclude patterns in there (I think that should work). Just keep in mind that ag doesn't support double-star patterns.

[EvgeniyMakhmudov]

Refresh in Embedded does not working. I must execute "reload window" of vscode to todo-list process new config file.
New release have also another bugs.

[fabiospampinato]

I must execute "reload window" of vscode to todo-list process new config file.

@EvgeniyMakhmudov can you open a separate issue about that? Let's see what can be done. We are currently caching some values for performance reasons.

[fabiospampinato]

I've just pushed v4.1.1, I think everything mentioned here has been fixed (check the changelog for more info) except maybe the bug that causes files from node_modules to be searched, can somebody test if that's still a problem?

The release is still not perfect though, there's at least another little problem to fix, but I'll be busy during the next ~24h so I'll work on that later.

[ghost]

Everything seems to be working now!

[muuvmuuv]

@fabiospampinato just for clarification: I will now use the default ag engine and put my excluding files and directories in a .ignore and the include is not working, because that only works for javascript? Because I do not get any results...

So this is my .ignore:

/.git
/node_modules
/vendor
/logs
/cache
/dist
/assets
/tmp
/administrator
/bin
/components
/images
/includes
/jdownloads
/language
/layouts
/libraries
/media
/modules
/plugins

Will this override or extend the "todo.embedded.exclude" option? Because some of the globs set there are good and I do not want to add them again (and as you said double stars are not working).

[fabiospampinato]

Everything seems to be working now!

Nice.

@fabiospampinato just for clarification: I will now use the default ag engine and put my excluding files and directories in a .ignore and the include is not working, because that only works for javascript? Because I do not get any results...

Can you open a separate issue about this please?

[Sap333]

Still doesn't work if one tries to exclude top level directory of the project. I mean that if you have a "local" directory in the projects root then "/local/","local/**","local/" etc. don't exclude anything.
I check on "javascript" and default providers. Had to stop using the extension.