Bugzilla – Full Text Bug Listing
| Summary: | add API to allow translation units to provide the path that FileContent uses | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Chris Recoskie <recoskie> | ||||
| Component: | cdt-core | Assignee: | Project Inbox <cdt-core-inbox> | ||||
| Status: | RESOLVED INVALID | QA Contact: | Doug Schaefer <cdtdoug> | ||||
| Severity: | blocker | ||||||
| Priority: | P3 | ||||||
| Version: | 7.0 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
The suggested API does not provide additional information. With the given implementation it can only have an effect, when you illegally implement ITranslationUnit. If you do that, you can alternatively override the two methods that use FileContent.create(...), namely getAST(..) and getCompletionNode(...) to achive a different behavior. In any way it makes sense to specify how FileContent.create(...) shall behave when the location of the translation unit is a non-file URI. This might also solve your problem. The current behavior is: with file-location: return content; name: absolute file path. with uri-location or without location: return content; name: name of unit. The intention of the current implementation was: with file-location: return content; name: absolute file path. with uri-location: return null. without location (new editor): return content; name: name of unit. We could change that to: with file-location: return content; name: absolute file path. with uri-location: return content; name: string representation of uri without location (new editor): return content; name: name of unit. (In reply to comment #1) > The suggested API does not provide additional information. With the given > implementation it can only have an effect, when you illegally implement > ITranslationUnit. If you do that, you can alternatively override the two > methods that use FileContent.create(...), namely getAST(..) and > getCompletionNode(...) to achive a different behavior. Yeah, that seems to be an easy enough approach. I have done that now. I'll withdraw the patch. > In any way it makes sense to specify how FileContent.create(...) shall behave > when the location of the translation unit is a non-file URI. This might also > solve your problem. > The current behavior is: > with file-location: return content; name: absolute file path. > with uri-location or without location: return content; name: name of unit. > The intention of the current implementation was: > with file-location: return content; name: absolute file path. > with uri-location: return null. > without location (new editor): return content; name: name of unit. > We could change that to: > with file-location: return content; name: absolute file path. > with uri-location: return content; name: string representation of uri > without location (new editor): return content; name: name of unit. I can work with the existing create() methods since I can override where they are called from getAST(...) and getCompletionNode(...). However, I suppose that changing things as you suggest would not hurt the situation either. I never did fully get EFS parsing/indexing working though when I tried a couple of years ago, so this would not really get used right now by anyone. But maybe it's a good investment for the future. (In reply to comment #2) > I can work with the existing create() methods since I can override where they > are called from getAST(...) and getCompletionNode(...). However, I suppose > that changing things as you suggest would not hurt the situation either. I > never did fully get EFS parsing/indexing working though when I tried a couple > of years ago, so this would not really get used right now by anyone. But maybe > it's a good investment for the future. I'll leave the method as it is then. To get it right for non-file URIs, we'd need to extend the IASTFileLocation to use URIs or even IIndexFileLocations instead of the plain String. I realized, that I need to prevent the project indexer from attempting to index translation units with non-file locations. It would not be able to compute correct IndexFileLocations off the information in the AST. |
Created attachment 168394 [details] proposed patch The change that deprecated CodeReader ended up breaking remote index related features, as in order for there to be a proper mapping between AST names and index names, their locations must point to the same file. The FileContent class in its current form populates the AST location with just the name of the file if tu.getLocation() is null, which it always will be for remote EFS files. Since the file locations used by the index contain absolute path information, searching for such names in the index will always fail. The reason this worked OK before is that RDT has its own ITranslationUnit implementation which knew how to create a code reader with the proper path as it's seen on the remote system. My proposed solution is to add an API to ITranslationUnit for the purpose of retrieving the path that FileContent should use. This way local translation units can still return the local path, but remote translation units can return the path on the remote machine. Proposed patch attached that improves upon the previous version that was too hastily checked in.