godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Rework `CodeEdit` API for code completion to be easier to use #10550

Open HolonProduction opened 3 months ago

HolonProduction commented 3 months ago

Describe the project you are working on

Godot

Describe the problem or limitation you are having in your project

The usage of CodeEdits completion functionality is not well documented, in an attempt to fix this, I found the interface to be confusing and not easy to use.

Overall the design doesn't match the expectations that users have when interacting with the feature (especially for completion prefixes as seen in godotengine/godot#73968 )

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Overall: My main problem is that we have a signal and a virtual method which share a name but not a purpose. Most users want godot to trigger completion when necessary and just populate the options, this use case fits the signal. The virtual method can be used if godots checks for triggering are not sufficient for the use case.

To make this clearer to the user, those two purposes will be split into two separate names now. Checks whether completion should be triggered will now be done by check_for_code_completion which comes with a default implementation which leverages the completion prefixes. If those checks are not enough users can implement them on their own with the new virtual method _check_for_code_completion. In contrast to before, this method has no responsibility for populating the options, it will just call request_code_completion when necessary. Checks can be skipped by calling this method directly (which should be done when using the shortcut).

request_code_completion has now only one job, to list options.

More specific details:

Those checks have two purposes, to cite from the documentation:

Otherwise will check that the caret is in a word or in front of a prefix.

Those checks are meant to regulate when completion is triggered when typing in the code edit. Those checks will now be performed by check_for_code_completion.

Will ignore the request if all current options are of type file path, node path, or signal.

This seems a bit arbitrary and I guess it was tailor made at some point, to make argument option completion possible for GDScript. This is not required anymore though since the GDScript Language handles this on its own now (as can be seen when triggering autocompletion in such a situation with the shortcut, and thus bypassing the checks).

request_code_completion(false) will be replaced by check_for_code_completion.

The p_force parameter will always be true. Users can use it like a method that is connected to the signal.

Note: Another option would be to deprecate the signal and go all in on the virtual method, I actually prefer this way, since with the signal we give users the option to shoot themselves in the foot by connecting multiple methods (this won't work as expected since every call to update_code_completion_options will erase the previous options). But that would mean to only deprecate the parameter which might feel strange to users.

(It will call to request_code_completion if the shortcut is used)

This is currently automatically done and the prefix array might add additional symbols. This behavior is not well documented and hard for users to find. The checkbox makes clear that characters don't need to be added as prefixes, and also gives users an option to disable it and only trigger with specific characters.

This propsoal does break compat in some ways, but a lot of existing code might just keep working, and if it doesn't, adapting is only a matter of moving some stuff to a new virtual method.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

A user will be able to have something working with this minimal example:

extends CodeEdit

func _ready():
    code_completion_enabled = true
    code_completion_requested.connect(_on_code_completion_requested)

func _on_code_completion_requested():
    var players = ["player1", "player2"]
    for player in players:
        add_code_completion_option(CodeCompletionKind.KIND_PLAIN_TEXT, player, player)
    update_code_completion_options()

Internally it is mostly renaming some stuff, moving some stuff between methods and rewriting and removing some checks.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Renaming stuff to make it easier to understand:

Connecting automatically:

Disabling chars as completion prefixes:

Is there a reason why this should be core and not an add-on in the asset library?

Usability on a builtin node

HolonProduction commented 2 months ago

Also if we decide to change stuff in this API it would be a good time to evaluate whether the p_force parameter on update_code_completion_options is really needed. From my reading it just adds a special case were completion is canceled if the cursor is behind a brace (.

Once again this feels a bit arbitrary and was probably also a solution for some GDScript use case (the argument hint for methods does not show when there are options, so after an open brace we don't want to show options so that users see the hint. But in the case of argument options we want to trigger, and to do that we set p_force.)

We should test whether the GDScript Language handles this case gracefully on its own (which it should)