Skip to content

Conversation

titoBouzout
Copy link

@titoBouzout titoBouzout commented Jul 3, 2025

Or something on these lines would be enough I suppose.

Comment on lines +559 to +561
Reloads all folders in the current project and updates the side bar. Note:
While the command runs in the background without blocking the app, it will
still delay any pending update to the sidebar until completes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like very specific info that I believe to be rather intuitive behavior and that I would not consider to be worth mentioning, honestly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When searching GitHub to see how people use this api, I do not see set_timeout being used for cases that I know will delay the sidebar from being updated.

So I conclude that refresh_folder_list delaying the sidebar is unexpected and not intuitive.

The confusion may come from that the API call runs async without blocking the application, but I personally (as others it seems) never noticed that that will also "reverse" the queue of operations leading to the delay.

To be roughly specific,

  1. create a folder
  2. run refresh_folder_list

What happens is that 1 will run only after 2 is done. What one expects is:

  1. create a folder
  2. file system events add the folder to the sidebar
  3. refresh_folder_list scans the whole thing and adds the folder in the case it wasnt added in step 2

I understand, step 2 is somewhat skipped, given that refresh_folder_list was triggered, possibly destroying the event listeners, can't really tell what happens, but an unexpected delay happens.

Besides the accuracy of the technicalities, it's an issue of expectations. I understand you beg to differ, do as you consider.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid I don't understand this message at all. What is your use case? Why is set_timeout being used, for what purpose and when exactly? What other operations are involved here? For your order of events, when exactly is the refresh_folder_list command called and how are the two different step lists comparable when they don't have the same amount of input triggers?

To me, it sounds natural that triggering a folder refresh would start a background job, which, after it concluded, causes the sidebar state to be refreshed with the newly scanned data. In some applications, the file tree may be repainted more aggressively as soon as new files/folders are found, but waiting until the scan has completed is also fairly common and reduces flickering on many quick updates. Either way, listening to new file changes to display them immediately while you are already refreshing the sidebar seems somewhat rendundant. Of course, you still need to listen to any changes made after you walked the filesystem at that position, but that is an implementation detail and, as far as I understand, not related to the issue you describe.

With all this in mint, I fail to see how your explanation makes the behavior of refresh_folder_list performing an asynchronous operation in the background, unexpected or an obstacle you somehow need to work around with.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a folder shows the folder in the sidebar in an instant.

But when you create a folder and trigger an update, it won't be instant, because first, it needs to refresh the whole tree. The issue raises that refresh_folder_list doesnt just refresh the visible folders/files in the sidebar, it crawls the whole thing, even invisible things because of goto anything and who knows what else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants