-
-
Notifications
You must be signed in to change notification settings - Fork 62
add aditional info to refresh_folder_list
#133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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,
- create a folder
- run refresh_folder_list
What happens is that 1 will run only after 2 is done. What one expects is:
- create a folder
- file system events add the folder to the sidebar
- 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Or something on these lines would be enough I suppose.