-
Notifications
You must be signed in to change notification settings - Fork 313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#6664,#505]: Enable External Workspace completions and tests - 5/n #6730
[#6664,#505]: Enable External Workspace completions and tests - 5/n #6730
Conversation
Also add a CharFilter that enable: * '@' to filter the lookup elements and not hide the lookup. * '/' to autocomplete selected item Other issues: bazelbuild#505
endIndex = rawText.length() - 1; | ||
endIndex = rawText.length() - quoteType.quoteString.length(); | ||
} else { | ||
endIndex += 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still don't cover this scenario, however I'm not sure if we even should
Screen.Recording.2024-09-05.at.11.23.41.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea. I don't think we should do that here.
|
||
WorkspaceRoot workspaceRoot = WorkspaceHelper.getExternalWorkspace(myElement.getProject(), name); | ||
if (workspaceRoot != null) { | ||
return BuildReferenceManager.getInstance(myElement.getProject()).findBuildFile(workspaceRoot.directory()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior is a bit different as it points to a BUILD file, but it's out of scope of this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea. This is slightly wrong, too, since it should point to either a WORKSPACE
or a MODULE.xx
file anyway. It's a net improvement though and is still helpful for the next step: to root completion of packagePaths from the containing folder
return EMPTY_ARRAY; | ||
@Nonnull | ||
public BuildLookupElement[] getVariants() { | ||
BlazeProjectData blazeProjectData = BlazeProjectDataManager.getInstance(myElement.getProject()).getBlazeProjectData(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried it could kill the UI in case of a large number of external repositories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should come from in-memory data though.
Checklist
Discussion thread for this change
Issue number: #6664, #505
Description of this change
Also add a CharFilter that enable:
* '@' to filter the lookup elements and not hide the lookup.
* '/' to autocomplete selected item