Gruntfuggly / todo-tree

Use ripgrep to find TODO tags and display the results in a tree view
Other
1.4k stars 135 forks source link

Remove File Watcher functionality #723

Closed Gruntfuggly closed 1 year ago

Gruntfuggly commented 1 year ago

This functionality is a constant source of problems. It doesn't really add anything useful and many people enable it without realising what is does or how it works.

The only reason it has any use is if files are created, modified or deleted outside of vscode.

The functionality will be removed in the next version of the extension unless anybody has some compelling reason why it needs to be kept.

joshhanley commented 1 year ago

@Gruntfuggly Thanks for a great extension!

I'm assuming this is for watching file changes outside of VSCode while VSCode is running? But if this feature is removed, will todos added elsewhere automatically be picked up on on a restart of VSCode during the initial indexing? If so, then yeah I don't see any issues.

Luk164 commented 1 year ago

@Gruntfuggly Thanks for a great extension!

I'm assuming this is for watching file changes outside of VSCode while VSCode is running? But if this feature is removed, will todos added elsewhere automatically be picked up on on a restart of VSCode during the initial indexing? If so, then yeah I don't see any issues.

^This, if they get refreshed on reload of VSC it should be fine. Also maybe add a button to manually refresh everything without the need to reload VSC itself. Just a small refresh icon on the window should be more than enough.

mikestopcontinues commented 1 year ago

I can see this being a problem on git pull. My team uses TODOs to pass work between timezones.

What do you think about adding a refresh interval? Does VSC offer any hooks you can use to help mitigate some of the problem situations?

Gruntfuggly commented 1 year ago

The workspace gets scanned on reload (assuming the scan mode includes the workspace - which is the default).

Pulling is a good case. I'll do some experiments to see if there is a nicer way to trigger the refresh under these circumstances rather than using the file watcher. One easy option would be to add a rescan on refocus of the window - but that won't work if you do the git pull from within vscode itself.

You can do a manual refresh at any time from the button in the tree view, but that's not so convenient.

The main problem with the file watcher is that the default setting is to watch all files in the workspace, which can easily put a massive burden on the extension host - and there's no obvious simple alternative.

nabil1440 commented 1 year ago

Making it to watch specific directories would be a convenient option.

Gruntfuggly commented 1 year ago

That's already an option (todo-tree.general.fileWatcherGlob), but most people seem to just enable it for the whole workspace.

hfisaquiel commented 1 year ago

Todo is for a post action, after the main activity was made. I don`t see problem on remove persistent watcher.

jwarkentin commented 1 year ago

Pulling is a good case. I'll do some experiments to see if there is a nicer way to trigger the refresh under these circumstances rather than using the file watcher.

Another situation that would be nice to address is switching branches.

With that said, I think it's still beneficial to remove the file watching even if there isn't a solution in place for those situations yet. I don't mind clicking the refresh button in the meantime if I need to. I'm far more interested in reducing the load on my system. But if there's not a way with a VSCode API you could always just poll and see if the git HEAD ref changes. That would cover pulling and branch switching.

Gruntfuggly commented 1 year ago

That's not a bad idea - it would probably do 99% of what people want from the file watcher with virtually no overhead.

Gruntfuggly commented 1 year ago

The next release will have a simple mechanism that polls the HEAD ref. If it changes it will refresh the tree.

The setting todo-tree.general.autoGitRefreshTimeout sets the polling interval in seconds - set it to 0 to disable altogether.

mikestopcontinues commented 1 year ago

Very cool. Thanks.

sql-sith commented 1 year ago

So, to summarize - polling the HEAD ref is a way to refresh the view for branch switches and pulls. Also, it sounds like everything is scanned when the workspace is opened.

I think that only leaves the use case of a change being made outside of VSCode after the workspace is opened, and the way to display those items is a manual refresh. Could we add a setting for an automatic manual refresh every X minutes? That way, people could still have a full "set it and forget it" experience. There would just be a delay in showing out-of-editor changes.

Or ... would that just be overkill for a niche use case?

Gruntfuggly commented 1 year ago

TBH, it feels like it could be a bit overkill - my worry is that users may accidentally set the interval too low and end up hogging the CPU in the same way the file watcher does at the moment.

cleonard-godaddy commented 1 year ago

I agree that if this was implemented, it should have a safe minimum period so as not to reproduce the performance issues you're describing.

It sounds like there isn't anybody else thinking this would be important. I've come to agree that this is just a niche use case, so it's probably not worth doing at this point.

cleonard-godaddy commented 1 year ago

In case it's confusing, I'm also sql-sith - this is my work account. I'm on my phone right now so I'm going to leave these comments under my work account instead of figuring out how to tell GMail to switch GitHub accounts.

eestrada commented 1 year ago

If the "automatic manual refresh" setting can only be specified in minutes (not seconds) that solves the issue of people inadvertently overtaxing their system; at most they can poll once per minute. Again, setting the value to 0 would just disable "automatic manual refresh" altogether . I do think that solves the remaining use cases.

Alternatively, just setting a hard floor ("this setting cannot be lower than 5 minutes") and documenting it in the settings help would again solve the issue.

Gruntfuggly commented 1 year ago

OK - I'll just add a periodic refresh in minutes (good idea). I think that should cover everything then 😄

Gruntfuggly commented 1 year ago

Just uploaded a new version with automaticGitRefreshInterval and periodicRefreshInterval.

lonix1 commented 1 year ago

This was a very good idea!

Is there any old setting we need to delete or reconfigure, or can we just upgrade without making any changes?

Gruntfuggly commented 1 year ago

If you were previously using the file watcher then you can remove the old setting(s) if you want (they'll be ignored) and either enable the periodic refresh or the automatic Git refresh depending on your use case.

lonix1 commented 1 year ago

Thanks. For reference, the old settings are:

todo-tree.general.enableFileWatcher
todo-tree.general.fileWatcherGlob

And the new settings are:

todo-tree.general.automaticGitRefreshInterval
todo-tree.general.periodicRefreshInterval