lszomoru
commented
11 months ago /gifPlease
Closed eleanorjboyd closed 11 months ago
lszomoru
commented
11 months ago /gifPlease
vscodenpa
commented
11 months ago Thanks for reporting this issue! Unfortunately, it's hard for us to understand what issue you're seeing. Please help us out by providing a screen recording showing exactly what isn't working as expected. While we can work with most standard formats, .gif files are preferred as they are displayed inline on GitHub. You may find https://gifcap.dev helpful as a browser-based gif recording tool.
If the issue depends on keyboard input, you can help us by enabling screencast mode for the recording (Developer: Toggle Screencast Mode in the command palette). Lastly, please attach this file via the GitHub web interface as emailed responses will strip files out from the issue.
Happy coding!
eleanorjboyd
commented
11 months ago lszomoru
commented
11 months ago Re-uploading the complete video for reference.
https://github.com/microsoft/vscode/assets/3372902/b691bf2e-569a-4d71-a4c2-d9e2d8c9f2f0
joaomoreno
commented
11 months ago Simple repro:
git init
mkdir -p a/b
mkdir -p a/c
touch a/b/b.txt
touch a/c/c.txtThen in the SCM view, make sure no resource is selected (since it would trigger the auto reveal feature) and stage one file at a time:
https://github.com/microsoft/vscode/assets/22350/1ee2525a-917b-4619-9565-725ffab9571f
abhijit-chikane
commented
11 months ago I am seeing this in explorer view in todays insider
Steps to reproduce
Version: 1.85.0-insider (user setup) Commit: c6b48c364dde70c12bf977f1da50acda0fd801e3 Date: 2023-11-30T05:35:40.559Z Electron: 25.9.6 ElectronBuildId: 25427645 Chromium: 114.0.5735.289 Node.js: 18.15.0 V8: 11.4.183.29-electron.0 OS: Windows_NT x64 10.0.22621
pdanpdan
commented
11 months ago Same as above.
VS Code version: Code - Insiders 1.85.0-insider (c6b48c364dde70c12bf977f1da50acda0fd801e3, 2023-11-30T05:35:27.138Z) OS version: Linux x64 6.5.6-76060506-generic Modes:
joyceerhl
commented
11 months ago I still see the collapsing, and in addition the collapsed node is no longer expandable. Also, after unstaging the staged folder, I can no longer restage it
https://github.com/microsoft/vscode/assets/30305945/d5a2ba0f-e001-4519-8bae-3bbfc4f28036
pdanpdan
commented
11 months ago After latest changes things are broken big way:
Version: 1.85.0-insider Commit: 621d89f2ef4b7715f681384739820cf08dfeeecd Date: 2023-12-01T01:38:16.445Z Browser: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Code-Insiders/1.85.0-insider Chrome/114.0.5735.289 Electron/25.9.6 Safari/537.36
joaomoreno
commented
11 months ago @pdanpdan We've confirmed it. We've already frozen the Insiders build to prevent more users from getting it. We're getting fixes in and expect to have a new Insiders build in a short while.
joaomoreno
commented
11 months ago Fixing such a tiny bug has suddenly uncovered a complicated mesh of things. I'll do my best to tell the story here:
The latest additions to the SCM view made it move from an object tree to an async data tree. The move uncovered a bug in how the async data tree handles collapse state in compressed trees. My first attempt to fix this was https://github.com/microsoft/vscode/pull/199591. My fix was incomplete and caused undesired behavior.
At the same time, @lszomoru looked into using the diffIdentityProvider tree feature to fix a performance regression, with https://github.com/microsoft/vscode/pull/199679. Part of @lszomoru's fix contains a fix in tree infrastructure in which an options bag just wasn't properly being passed around. Any compressible async data tree that attempted to use diffIdentityProvider simply wasn't getting it working at all because the provider wasn't passed around to all the layers. Before this, only another tree was both compressible and async and used diffIdentityProvider: the Explorer. Back in 2021, @isidorn adopted this with https://github.com/microsoft/vscode/commit/9b1d85bad8cd752c4657627424edfa58dd5ce4aa, but did not recognize that the option didn't work. Now that the diffIdentityProvider is leveraged by the Explorer view, bugs appear:
https://github.com/microsoft/vscode/assets/22350/4feb14e8-de46-4f62-8372-549e3b1aaa7a
But why do twisties disappear? I've traced it down to https://github.com/microsoft/vscode/commit/a8dd7f60a6204353e8d6969c8bc39f0476d46e2a, a change by @connor4312 down in the index tree model also from 2021. This change doesn't look good to me since it defines that nodes should not be collapsible if they have no children... empty Explorer folders are good counter examples: they are collapsible elements with zero children.
To fix today's Insiders, we've frozen the broken build and will release a new build with https://github.com/microsoft/vscode/pull/199756 which reverts to the initial state of not having diffIdentityProvider working in the Explorer. Note this is identical to simply removing diffIdentityProvider from the Explorer's updateChildren call. This second Insiders build will have a stable Explorer state and a functioning SCM state with the exception of this issue's bug.
We have three options to move forward:
Regardless, we should follow up on the rest:
diffIdentityProvider from the Explorer's updateChildren call or follow up with whatever bugs might arise from this adoption. @connor4312 @lramos15 
connor4312
commented
11 months ago Good findings, Joao! As mentioned in the comment on that issue, I think that commit is good to revert. Reverting it doesn't seem to have negative effects in trees that use the diffIdentityProvider, as far as I can tell. Unfortunately I don't recall why I initially had it added.
Please let me know if any further issues are observed with diffIdentityProvider. It should work for the explorer and would make rendering more efficient there as it does elsewhere. I'm happy to help with any difficulties encountered during the adoption.
The main footgun generally found during its adoption, which the explorer may encounter, is cases where a tree is implicitly expected to rerender elements which have the same identity when setChildren is called. The general way I deal with this is having emitters or observables on the items which are listened to by the renderer to update any visible templates.
Testing #199172
while in tree view everything looks good until I have 3 files nested under a single folder with sub folders in between. When I stage and unstage these changes in different configurations sometimes it collapses the block of folder and sometimes it doesn't. Also when it unexpectedly collapses the folders, the root folder switches from being
src/client/testing/testControllertosrcthen re-expands when I click on it. If I only click to collapse a block at the root folder it stays assrc/client/testing/testController. So the two bugs I can see are the collapse when staging/unstaging for some folder arrangements and when block accidentally collapse they have the wrong root node name.(I have a video but I can't upload it rn, will try again later)