Add external editor presets

[opl-]

Allows filling in external editor exec_path and exec_flags settings by choosing from a list of presets.

Original

You can choose a preset to set exec_path and exec_flags to values for the chosen editor. Optionally, if your editor is not in PATH, you can change the exec_path to a non-empty path to override the preset, while keeping the preset exec_flags value for the selected editor.

Choosing "Custom" (default) as the preset uses the old behavior.

[capnm]

?

			case 7: {
				path = "codium";
				flags = "{project} --goto {file}:{line}:{col}";
			} break;
			case 8: {
				path = "code";
				flags = "{project} --goto {file}:{line}:{col}";
			} break;

https://vscodium.com/ ❤️

[Calinou]

@capnm Since VS Codium and VS Code use the same command-line arguments, I think it would be better to let the user override Exec Path themselves if they use VS Codium.

[opl-]

Admittedly I wasn't sure where to draw the line, so I simply took the editors already mentioned in the documentation as the base group. I'd love to hear what others think we should include. I also agree that in case of VS Codium, which is practically a fork of VS Code, simply entering vscodium as the path is an easy change and should hopefully be reasonable to VS Codium users, though I'm open to other opinions.

Other editors that I considered, but wasn't sure about adding:

• dte
• emacs (I mean, we already have vim!)
• MonoDevelop (C#)
• nano
• Notepad++
• Visual Studio (C#)

The C# ones are going to be mostly useless in non-Mono builds, but I felt like changing the options based on whether mono is enabled for an editor setting might be weird. Either the numerical values wouldn't match between the builds or the alphabetical order would be broken. Suggestions welcome.

Notepad++ can only really be ran on Windows (at least without wine).

dte and nano might be too simple, but different people use different tools.

[capnm]

The current selection of the text editor is a bit strange, it should also update the corresponding fields/labels (defaults) in the settings.

[opl-]

@capnm I initially explored the idea of updating the exec_path and exec_flags settings when a preset is changed, but I realized that I couldn't easily track which settings changed and that the editor settings dialog might not even update after I change them, so I originally abandoned that idea. You prompted me to finish implementing that version and... It feels a little laggy. Recording below. If anyone wants to take at the code, I pushed it to a separate branch: https://github.com/opl-/godot/tree/feat/external-editor-presets-alt

[capnm]

Yeah, that's weird too ;) Is it somehow possible to change all the values and then do an update?
Otherwise (if possible without a lag) you could probably enable the path and flags controls for the custom entry only.

[setanarut]

Exec Preset Should be on top hierarchically.

1. Use External Editor
2. Exec Preset
3. Exec Path
4. Exec Flags

[opl-]

@capnm Right, so the delay seems to be deliberately caused by the Editor Settings Dialog with a 1.5s timer started when the settings change (source).

I presume the delay is there to prevent the editor from updating the UI several times in case multiple settings are changed one after another. Changing the timer delay to 0.1s feels more responsive but still visibly laggy, with the delay seemingly fluctuating. Another problem is that if the user changes exec_path and "confirms" the value by clicking on the input for exec_flags, then the cursor in the text input gets reset to the beginning after the inspector gets updated. I assume this is because the EditorSettings object doesn't inform the EditorInspector used by the Editor Settings dialog which properties changed, forcing it to redraw the entire input (similar to what happens in the normal Inspector dock with some properties; see #25046).

Other options:

• Manually trigger updates after changing the settings to bypass the timer: could work, but still resets the cursor position,
• Make the settings read only: impossible, because the inspector only allows making all the properties read only,
• Hide the settings: possible, but requires storing and then changing the usage of the relevant PropertyHint object, which I'm pretty sure is not a standard practice (and would require some extra functions or public fields since the property is declared by ScriptEditorPlugin, but the settings changes are handled by ScriptEditor).

Overall I feel like the EditorSettings object isn't really designed to allow the settings to affect each other. As far as I can tell no setting actually affects how the others are presented in the interface: even looking at the context of this PR, the "Use external editor" setting doesn't hide the "Exec Path" and "Exec Flags" properties. I could try looking into improving this situation later if I have some time, but for now it feels out of the scope of this PR.

---

@Hazarel As for the order: the settings are already declared in the appropriate order, but since I had an older config version that didn't include the exec_preset setting, the new setting was added to the end of the EditorSettings property list and so appears at the end. For someone who starts the editor with fresh settings the section looks like this:

[Calinou]

This is a good feature, but I'm concerned about this not working out of the box on Windows and macOS. On those operating systems, most editors/IDEs are not added to the PATH environment variable (with the exception of Atom and Visual Studio Code).

[opl-]

Users are welcome to change the path to the editor's executable while still getting the benefits of Godot filling in the Exec Flags field for them. This means it's no longer necessary to look them up in Godot's or the editor's documentation.

[Calinou]

Users are welcome to change the path to the editor's executable while still getting the benefits of Godot filling in the Exec Flags field for them.

I'm still worried about users expecting the default paths to work out of the box and reporting issues here 🙂

[opl-]

I already submitted a companion PR that updates the docs to explain how the settings work in an attempt to combat this.

I'd also argue that right now, by forcing users to do this manually, we are making the experience worse and creating an opportunity for them to make mistakes. For users like me, who previously used the function, they likely had to look up what the value for Exec Flags should be every time they're setting up Godot (whether it be after an update or during engine development). For new users, this makes it easier to miss that Godot allows opening not only the file, but also the project directory and a specific line + column.

I agree that some of it could be more intuitive, but as I explained above with the way the settings dialog works right now it's difficult to make it so. In the end, I believe that this should allow wasting less time on setup until a better solution comes along.

[akien-mga]

Sorry for the late review.

The feature makes sense, one thing we're not too sure of is:

• The list is arbitrary and some users will inevitably find that their favorite editor is missing.
• The list is integer based so we can't reorder options without breaking compat, and some users might complain about the chosen order.

It should probably be refactored so that it's string based, so that we can order them alphabetically (no favoritism) and add new entries without breaking compat.

[opl-]

Had another go at implementing this the way I felt it should work, and this time I managed to find some code I could hook into which allowed me to do exactly what I wanted:

(Switching to Kate appears to fail in this recording because of #20708)

[opl-]

Resolved the merge conflict, and fixed a setting definition going missing at some point.

[Vennnot]

Can this and the linked issue be merged?

[Calinou]

Can this and the linked issue be merged?

The PATH issue on Windows/macOS is unfortunately still present. If presets are exposed, people will expect them to work without any configuration whatsoever, even if the user didn't check the "Add to PATH" checkbox in the installer for VS Code. Most other editors' installers don't add the editor to PATH either.

I think we should fix this by changing the default executable path depending on the current OS to match the default used by each editor. This will require some research (such as manually installing each editor to find its default executable path).

I know this is quite a lot of work, but it's important that presets truly work as intended on any platform.

[opl-]

I imagine that assumption will very quickly break down. What if the user installed the editor on D: instead of C:? How far should we go in an attempt to find these existing paths? The flip side is that if we just assume everyone installs the editor in its default location, it will appear as though the editor path is already configured and doesn't require any attention, even if it's wrong. If we start calling various APIs to figure out the actual location, that's potentially a whole lot of extra code to maintain just to determine where the program might be.

Additionally, I don't even have Windows nor Mac available to me, so I'm literally unable to implement this myself without going extremely out of my way to gather even the most basic defaults.

Ultimately, this PR has been stuck in limbo for over two years at this point, and while I understand the maintainers are busy, I'd like to see some resolution. I wasn't even aware these defaults were blockers for you as you never expressed that beyond describing it as a worry.

Personally, I think something is far better than nothing, as I find exec_path is far easier to determine than exec_flags. I think it's also worth noting that according to my testing, currently if the user sets an invalid exec_path attempting to open a script will simply do nothing, not even log an error anywhere, which makes me feel like the priority is being put in the wrong place.

[Calinou]

One alternative that sidesteps the issue with the path would be to make the presets only set the flags, but not the path.

I think it's also worth noting that according to my testing, currently if the user sets an invalid exec_path attempting to open a script will simply do nothing, not even log an error anywhere, which makes me feel like the priority is being put in the wrong place.

This is definitely worth a PR 🙂

I do see this error printed to the Output panel when the path is empty, but I think it should be turned into an AcceptDialog that will be more noticeable:

editor/plugins/script_editor_plugin.cpp:2316 - Couldn't open external text editor, falling back to the internal editor. Review your text_editor/external/ editor settings.

The fallback to the internal editor also doesn't work – the editor doesn't switch to the Script tab when double-clicking a script in the FileSystem dock.