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

Create .editorconfig file only on project creation #97270

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

timothyqiu
Copy link
Member

@timothyqiu timothyqiu commented Sep 21, 2024

Instead of always generating one when missing, this PR creates .editorconfig on project creation.

@timothyqiu timothyqiu added this to the 4.x milestone Sep 21, 2024
@AThousandShips AThousandShips changed the title Choose to generate .editorconfig file on project creation Add option to generate .editorconfig file on project creation Sep 21, 2024
Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Yes, this doesn't fix the Visual Studio issue retroactively for existing projects, but it's probably better to let users decide whether they need .editorconfig or not.

Also please remove the following lines:

# This file is required to workaround `.editorconfig` autogeneration (see #96845).

const String editor_config_path = path.path_join(".editorconfig");
Ref<FileAccess> f = FileAccess::open(editor_config_path, FileAccess::WRITE);
if (f.is_null()) {
ERR_FAIL_MSG("Couldn't create .editorconfig in project path.");
Copy link
Member

@dalexeev dalexeev Oct 4, 2024

Choose a reason for hiding this comment

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

Maybe we should use ERR_PRINT instead? .editorconfig isn't so critical and EditorVCSInterface::create_vcs_metadata_files() result is also not checked.

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Oct 4, 2024
@akien-mga
Copy link
Member

I agree with changing this to a project creation step.

I'm not fully sold with the UI though, this dialog is already heavy and adding more checkboxes and labels really makes information overload IMO. It might also be an issue on the Android editor where the dialog might again become too tall for screens in landscape mode.

We might need to rethink this UI a bit, CC @Calinou @KoBeWi.

I'm fine with merging this as a first step and have others rework the UX later.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 4, 2024

There are a couple of options I can think of, if we don't want the dialog to be too high:

  • Move checkboxes on the right, making the dialog wider
  • Add foldable categories for Renderer and Metadata files
  • Use TabContainer

@bruvzg
Copy link
Member

bruvzg commented Oct 4, 2024

Not sure if I see any reason for making it optional. It's tiny. And .editorconfig in the form Godot creates should not have any effect on most IDEs (it's only setting encoding, and for Godot projects only supported is UTF-8).

Also, adding this option might create a false impression that Godot supports reading and using .editorconfig (which is probably a good idea, but currently it is not implemented), no one will check tooltips.

@dalexeev
Copy link
Member

dalexeev commented Oct 4, 2024

@bruvzg I think we could hide the option from the project creation dialog for now and create the file without asking.

But in my opinion it still makes sense to move this step to project creation. Since the inability to delete the file permanently seems to be annoying for some users. And it also has a small chance of unexpected issues in projects.

Not sure if it's related but there is ERROR: Failed to create file "res://.editorconfig".:

@akien-mga
Copy link
Member

I agree with not making it optional, but having it only on project creation. Forcefully creating the file when editing pre-existing projects was indeed a concern of mine, and we've indeed see it appearing in unexpected places, like the GDScript test suite.

If some users ask for it, we could later add an editor setting to configure whether to create it, or expose it to the project creation dialog, but for now I think we can consider this is part of the new default files for a Godot project.

@timothyqiu timothyqiu requested review from a team as code owners October 5, 2024 06:24
@timothyqiu timothyqiu changed the title Add option to generate .editorconfig file on project creation Create .editorconfig file only on project creation Oct 5, 2024
@timothyqiu
Copy link
Member Author

Done. This PR now moves .editorconfig creation from every startup to project creation.

@Repiteo Repiteo merged commit f31a2cc into godotengine:master Oct 8, 2024
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 8, 2024

Thanks!

@timothyqiu timothyqiu deleted the optional-edconfig branch October 8, 2024 23:44
@aaronfranke
Copy link
Member

See also the previous discussion here: #96845 (comment)

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.

[4.4.dev3] .editorconfig file gets created in Godot project root directory
7 participants