Use els_utils:lookup_document for lookup #930
Conversation
sgillis
changed the title
There was a problem hiding this comment.
Hi! Even if in principle this makes sense, I am a bit worried about what would happen in case the project is misconfigured (which can happen often among new users). This change would cause crashes, which would result in providers stopping and the server malfunctioning. See comments below for a possible solution.
| ?LOG_WARNING("Failed lookup while expanding includes [uri=~p]", [Uri]), | ||
| [] | ||
| end. | ||
| {ok, Document} = els_utils:lookup_document(Uri), |
There was a problem hiding this comment.
What would happen if the included path is incorrect or if it does not exist yet? Or, more likely, if the current project is not configured correctly, so the included file is not found? That could cause the provider to crash, right?
| ?LOG_INFO("Lookup failed [Error=~p]", [Error]), | ||
| [] | ||
| end. | ||
| {ok, Document} = els_utils:lookup_document(Uri), |
There was a problem hiding this comment.
Same here. If a project is misconfigured, some dependencies will not be found and a hard pattern match here would result in a less stable server. Maybe we want to modify the els_utils:lookup_document/1 so that if it fails after indexing it returns an error and keep the case in the callers?
|
@robertoaloi Makes sense, I'll change that. |
|
Nice, thanks! |
Description
Use els_utils:lookup_document for document lookups to make sure that files that were not yet indexed are indexed on request. Related to #881
Fixes #928