Skip to content
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

Adds the ability to set a custom C# editor, to allow users to still use the built in Godot editor for GD scripts. #74517

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

dmitsuki
Copy link
Contributor

@dmitsuki dmitsuki commented Mar 6, 2023

This commit adds the ability, if using the dotnet editor, to add a custom command to launch whatever editor you want for c#, while keeping use of the built in editor for GDScript.

@dmitsuki dmitsuki requested a review from a team as a code owner March 6, 2023 22:09
@akien-mga
Copy link
Member

This commit adds the ability, if using the dotnet editor, to add a custom command to launch whatever editor you want for c#, while keeping use of the built in editor for GDScript.

I'm not sure I understand, isn't this already how it works? When you configure a .NET editor, it's used to open C# scripts, but opening GDScripts would still go to the builtin editor (unless you configure an editor-wide custom external editor for all scripts).

@dmitsuki
Copy link
Contributor Author

dmitsuki commented Mar 7, 2023

Yes, this is how it works. However, you cannot use a custom editor currently. You can only use VScode, rider, etc. With this commit, you can still use these editors, but you can also set a custom command to use any editor you want:IE vim, sublime text, emacs.

Currently to do this, you have to change the global editor, which would make you not able to use the built in editor for gdcript. All this pr does is bring specifically the dotnet editor option to feature parity with the "external editor" option

@dmitsuki
Copy link
Contributor Author

dmitsuki commented Mar 7, 2023

What about the static check failed? It's a little confusing to me, so any help on getting that fixed would be appreciated.

@akien-mga
Copy link
Member

akien-mga commented Mar 7, 2023

There is extra whitespace at the end of this line:
IMG_20230307_224450

Note that before this is merged, you will also have to squash the commits. But you can wait for an approval first.

@RedworkDE
Copy link
Member

Also the formatting of the code in general doesn't meet the style guidelines either, so that is just just going to fail CI next, run dotnet format on the files you changed to fix that.
(check the various scripts in misc/scripts to run all these checks locally)

@akien-mga
Copy link
Member

Looks good! Could you squash the commits? See PR workflow for instructions.

@akien-mga akien-mga requested a review from RedworkDE March 9, 2023 07:34
@dmitsuki
Copy link
Contributor Author

Looks good! Could you squash the commits? See PR workflow for instructions.

Sure, once all issues are resolved and it's approved I'll do just that

@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 16, 2023

It's been approved already, so please squash. Make sure that the commit message is short but descriptive. "Add the ability to set a custom C# editor" is a good option.

Copy link
Member

@RedworkDE RedworkDE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The global file one is important (sorry my earlier suggestion was incorrect), but the other suggestion are just nits.

hasFileFlag = true;
}

arg = arg.ReplaceN("{file}", file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tho this change is kinda hacky, so we may just no support this for the C# editor (esp, since here the solution path would probably be more useful):

Suggested change
arg = arg.ReplaceN("{file}", file);
arg = arg.ReplaceN("{project}", project_path);
arg = arg.ReplaceN("{file}", file);

and with the other variable:

var project_path = ProjectSettings.GlobalizePath("res://");
if (project_path.EndsWith('/')) project_path = project_path[..^1];

@dmitsuki dmitsuki force-pushed the dotnet-custom-editor branch from 4adce82 to 87ebf2a Compare March 18, 2023 02:26
This allows users to still use the built-in Godot editor for GDScript.
@YuriSizov YuriSizov force-pushed the dotnet-custom-editor branch from 87ebf2a to c2b97ec Compare April 18, 2023 11:56
@YuriSizov
Copy link
Contributor

YuriSizov commented Apr 18, 2023

Rebased and applied suggestions on your behalf. Left out the last suggestion as it's something that you would probably want to discuss further and perhaps address with a future PR.

@YuriSizov YuriSizov merged commit 3ff3af4 into godotengine:master Apr 18, 2023
@YuriSizov
Copy link
Contributor

Thanks, and congrats on your first merged Godot pull-request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants