fix: TC-1631 Explicitly synchronize PSI file cached document content and disk file content to avoid analysis happens before changes is saved to disk. #197
Conversation
ruromero
left a comment
ruromero
left a comment
There was a problem hiding this comment.
The solution seems reasonable but I am worried about locking the threads. Please review my comments and let me know if you already considered the IntelliJ threading model
Also maybe it's better to check every 500ms instead of 200ms
| private static void waitUntilMemoryAndDiskContentSynchronized(PsiFile file, String filePath) { | ||
| Document document = PsiDocumentManager.getInstance(file.getProject()).getCachedDocument(file); | ||
| if (document == null) { | ||
| LOG.warn("Document is not cached for file: " + file.getName()); |
There was a problem hiding this comment.
does this need to be a warning or can be debug?
|
|
||
| Path path = Path.of(filePath); | ||
| long timeoutMillis = 5000; | ||
| long intervalMillis = 200; |
There was a problem hiding this comment.
I don't know if looping every 200ms and blocking the current thread is a good idea. I'm afraid this could affect other extensions or the IDE itself.
Can you take a look at the Threading model docs?
…and disk file content to avoid analysis happens before changes is saved to disk. Signed-off-by: Chao Wang <[email protected]>
soul2zimate
force-pushed
the
TC-1631_alternative_main
branch
from
46272e1 to
d6956a7
Compare
|
|
@ruromero I read some docs and realized that it should be able to explicitly save the document content from memory to disk. I updated the fix with this much simpler approach. Please review again. Thanks.l |
soul2zimate
changed the title
2 participants
This is an alternative fix for TC-1631.
Background: previous fix made in #161 reading PSI file cached document content from memory and copy it to temp folder for analysis to avoid inconsistent file content between memory and disk while user applies the quickfix and the analysis happens before changes is saved to disk.
However, it causes trouble when we implements some recent features:
e.g.
Therefore, this is another proposed alternative that allows analysis waits maximum 5 seconds until the memory and disk are synchronized( 5 seconds is long enough as IntelliJ files are auto-saved after content change).Therefore, this is another proposed alternative that explicitly synchronize PSI file cached document content and disk file content to avoid analysis happens before changes is saved to disk.