-
Notifications
You must be signed in to change notification settings - Fork 53
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
editorconfig: Remove unnecessary rules #433
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This will cause EditorConfig-enabled editors to insert a newline, where Visual Studio trims such newlines when modifying those types of files.
It will cause on and off final newline diffs depending on what tool was used to edit such files.
I strongly recommend to keep this one.
To reiterate, it's not about the VS templates only, but the IDE's editing behavior.
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.
No, this is not true. If no rule exists for this in editorconfig, then it's the editor's default behavior coming into play. For most editors it's based on auto-detection or defaults of the editor. The editorconfig spec defines no default for this type of rule, so having no rule at all is the most compatible setting.
VS doesn't trim. It handles mixed line endings transparently without modifying. Only when adding new lines it will choose a line ending style (from auto-detection or VS options settings).
No. If this rule isn't present, an editor normally doesn't make any change to it. (VS doesn't)
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.
I think this remark is not about EOL, but about insert_final_newline (newline at end of file). It might be good to keep it as @JunielKatarn suggested.
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 the remark is about newlines at the end of files, then the statement is still incorrect. Visual Studio does not trim any CR or LF at the end of files when editing.
VS leaves everything as is and that's the behavior that we need. On the contrary, the existence of those rules has caused the PR's from @JunielKatarn to include such changes to the end of files which we neither want nor need (assuming that the current state is how we want everything to be).
It's generally important to understand that removing such rules doesn't mean that the opposite would happen. Removing rather means that the editor won't make changes in that regard.
Also important: Visual Studio doesn't need any editorconfig rules at all. It works totally fine without any editorconfig in place.
It's also very unusual to have those EOL and EOF rules when using VS. Never seen that before, it's in none of the templates, nobody does that because there's no need for this in case of VS. It does these things right just out of the box.
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.
I'd say we should leave
insert_final_newline = false
in. These are specific VS formats. VS never adds final new lines and removes them when found. It is safe and reasonable to leave this setting in the editorconfig, so other code editors won't accidentially insert final new lines when it is their default.