Closed payne911 closed 9 months ago
@cpedlar-syn feel free to help reviewing this since it touches some logic introduced by you in #313
Getting a review from @Trias (author of #241) would probably be even more relevant.
@cpedlar-syn feel free to help reviewing this since it touches some logic introduced by you in #313
The part that I had touched in the past looks good and a lot of the cleanup looks good. I can't comment on the other parts that I'm not as familiar with.
👍
This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale
label is removed or commented.
3 weeks without activity is a very short window to automatically decide that a PR should be discarded. It's not like this repo is seeing an incredible influx of contributions: having them pile up could be good to indicate to others that there is indeed interest in improving it.
On my side, I'm certainly noting that the hours it took me to debug and fix this will simply go down the drain. That's definitely demoralizing. :( I now wonder how many other contributions were simply wasted due to this odd policy.
@payne911, we appreciate your contribution and apologize for the delay in getting back to you. Unfortunately, we've been occupied with other priorities recently but the good news is, we have plans to revive this project as we'll be working on a new Ballerina language plugin based on this client.
Regarding the stale policy, it's a platform-wide initiative, but we understand your valid concerns. I'll explore the possibility of disabling the stale policy for this repository(at least until we revive this project), to ensure that valuable contributions don't go unnoticed.
Thank you for your understanding and looking forward to your continued contributions :)
Many things were causing issues related to closing and reopening files (fixes #344)
Core changes are (I could be wrong on any of this, so please be careful when you review this... you can follow my investigation and train of thought in #344):
LSPFileEventManager#fileRenamed
was sending duplicatedidChangeWatchedFiles
notifications.EditorEventManagerBase#unregisterManager
was not appropriately cleaning up theSet<EditorEventManager>
.DocumentEventManager#documentClosed
to not hit the appropriate code path becauseEditorEventManagerBase.managersForUri
was not appropriately maintained. (In turns causing thedocumentOpened
to also be broken.)EditorEventManager
's constructor was erroneously trying to reuse a previousDocumentEventManager
.documentChanged
to be aborted because the associatededitor
had been disposed.DocumentEventManager
'suriToDocumentEventManager
was not useful anymore, thus all the related code was removed.This PR also fixes a bunch of misc stuff I encountered while digging around:
final
propertiesThe original issue (#344) also points out many things which would still be worth investigating. I highly encourage the maintainers to closely read through it.