Remove async file adding from program #278
Conversation
src/LanguageServer.ts
Outdated
| return this.documentFileResolver(pathAbsolute); | ||
| builder.addFileResolver({ | ||
| readFile: (pathAbsolute) => this.documentFileResolver(pathAbsolute), | ||
| readFileSync: (pathAbsolute) => this.documentFileResolver(pathAbsolute) |
There was a problem hiding this comment.
The language server protocol has a concept called TextDocument. Those objects are in-memory copies of the files. Basically every time you open a file, type in the file, etc, it gets streamed to the language server automatically and exists in memory. So there's nothing async to do with those.
| src: change.pathAbsolute, | ||
| dest: rokuDeploy.getDestPath(change.pathAbsolute, options.files, rootDir) | ||
| }); | ||
| if (await util.pathExists(change.pathAbsolute)) { |
There was a problem hiding this comment.
I realize: why do you test if the file physically exists instead of letting the file resolver(s) try to find it? Otherwise what's the point to the list of resolvers?
There was a problem hiding this comment.
The file resolver concept was created to support the language server's TextDocument design. Generally you'll only have two resolvers: the language server, and then the actual file system. We read the language server first, and if not there, go to the file system.
Since our language server has a separate process to handle not-saved transient files (I call them standalone files), we can safely assume that all other files are physically located on disk, regardless of whether they are currently opened in the editor or not, so this check works just fine.
Unless I'm missing something, I don't see any need to modify the file resolvers to support doesFileExist logic since it doesn't seem to provide any benefit. Do you?
Since the xml parser change removed the need for any async parsing, there's no benefit to having async file adding logic at all in the Program and downstream. Notable changes:
Program,BrsFile,XmlFile.ProgramBuilderstill uses async logic when loading files because it yields a lot of performance benefits.fileResolverlogic out ofProgramand intoProgramBuilderso the language server can continue to use the in-memoryDocumentstoresfileResolverinto an object withreadFileandreadFileSyncfunctions so we can use the proper function when needed.fileExiststopathExistsandpathExistsSyncto line up with the ones fromfs-extraI broke the commits up so you can see just the raw api changes in this commit, and then all the unit tests in this commit.