I think this might require a change in the way the keybindings editor works: i.e. if a change is made, issue a change event, independent from the underlying file itself. I think the settings editor does the same.

I think this might require a change in the way the keybindings editor works: i.e. if a change is made, issue a change event, independent from the underlying file itself. I think the settings editor does the same.

@alexdima we might need a way to refresh keybindings model

After a bit of discussion with Sandeep, we came to the conclusion that a change from me probably is the underlying reason for the recent flakiness in the test. Specifically, file watching is more delayed on startup ever since it moved to the shared process and if a component or test relies on file events alone, there is a higher chance of failure.

I have opened #142027 for me to revisit this and I can bring back this test in its old form when that happened.

Nevertheless: I believe that we should never use file events as the only source of truth for changes when changes are made from within VSCode. File events are meant to detect changes from outside VSCode or an external program, not when changes are made within VSCode. As indicated, the keybindings editor should follow a similar strategy as the UI settings editor and update the keybindings settings file through some kind of higher level service that also sends out change events.

I believe that we should never use file events as the only source of truth for changes when changes are made from within VSCode.

This sounds reasonable. But since we always use IFileService for writing files, shouldn't it simply emit events after it finishes writing to a file? It sounds like a lot of work for each piece of code that writes files using IFileService to both listen to change events from IFileService and then also emit its own logical change event.

Also, there are cases when users edit keybindings.json or settings.json files directly from a code editor. In the code editor we don't know that these file are special and should trigger configuration model updates.

Yeah, we already have the infrastructure for events like that called IFileService.onDidRunOperation and we currently do not emit events for write operations, but that would be easy enough for me to add.

You could then globally listen on that event as well as the file events to ensure you get hold of any updates.

Also, there are cases when users edit keybindings.json or settings.json files directly from a code editor

That is fine. Somewhere in the keybindings model code you already listen to file change events and now all you have to add is the additional listener for IFileService.onDidRunOperation.

I pushed a change to have a FileOperationEvent of operation: FileOperation.WRITE that provides its resource: URI. You can listen to this via IFileService.onDidRunOperation.

👏