-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Sort autocomplete/code completion options in a better way #58931
Sort autocomplete/code completion options in a better way #58931
Conversation
18ecfc9
to
82d4930
Compare
82d4930
to
0d21ce9
Compare
The implementation looks good to me. Just need to fix the issues pointed by Akien. |
Done by ordering options by their location in the code - e.g. local, parent class, global, etc.
0d21ce9
to
f9e1c09
Compare
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.
LGTM
Thanks! |
I did many changes to the script API in #59553 and this PR has broken them. I suggest reverting this PR first and make it redone on top of my changes. It also adds things to script_language.h that should not be there, like the sorting order for completion. This should go in editor, not in script_language.h |
} | ||
}; | ||
|
||
const int KIND_COUNT = 10; |
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.
If this is only used by the editor, it should not be here.
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.
Indeed, can be easily moved.
|
||
ScriptCodeCompletionOption() {} | ||
|
||
ScriptCodeCompletionOption(const String &p_text, Kind p_kind) { | ||
ScriptCodeCompletionOption(const String &p_text, Kind p_kind, int p_location = LOCATION_OTHER) { |
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.
Make sure to check that script_language_extension bindings to this are correct.
Kind kind = KIND_PLAIN_TEXT; | ||
String display; | ||
String insert_text; | ||
Color font_color; | ||
RES icon; | ||
Variant default_value; | ||
Vector<Pair<int, int>> matches; | ||
int location = LOCATION_OTHER; |
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.
Should be an enum.
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 use the PARENT_MASK
enum value as a flag to indicate it comes from a parent - but then the other bits in the integer are used to store the depth of the parent, so it is not strictly an enum value.
Closes godotengine/godot-proposals#4189
Closes godotengine/godot-proposals#99
Please read above proposal for more info and some example of what code completion currently looks like.
After changes in this PR:
Code Overview:
ScriptCodeCompletionOption
. It is an int, but has a corresponding enum, and more importantly one of the enum values is a mask, LOCATION_PARENT_MASK = (1 << 8),. This allows you, for example, to sort closer ancestors differently from more distant ancestors. The class itself will be value 256, while the class it inherits will have its location at 257, and then its parent at 258, etc.const int location = classes_processed | ScriptCodeCompletionOption::LOCATION_PARENT_MASK;
ScriptCodeCompletionOption::Kind
's are sorted has been added too.