Address EditorConfig violations#2905
Conversation
The newer wxl (for ru-ru) does not have a BOM and the XML specification says that BOM for UTF-8 is optional: https://www.opentag.com/xfaq_enc.htm#enc_bom With this change the charset is as set with EditorConfig.
They are patch files, although they have a different file extension.
Technically it is even allowed to mix indentations in one file: https://github.com/casey/just#indentation However, it violats the EditorConfig of the project.
To fix the violation against insert_final_newline = true. All the other .sha256sums files for example also have a final newline.
| @@ -1,4 +1,4 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
In the .wxl files and several other XML-lke files I only remove the UTF-8 BOM (as charset is utf-8 and not utf-8 bom). The ru-ru file was already without the BOM and for XML files the BOM is optional by definition.
| # For example, the indentation in lists depend on the length of the list marker | ||
| # (such as '- ' or '100. '), at least in the GitHub Markdown flavor: | ||
| # https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#nested-lists | ||
| # Therefore, pressing tab should insert a single space to encourage situative accurate indentations. |
There was a problem hiding this comment.
It is maybe a matter of taste if inserting only one space with a tab is a nice solution for this or not. Another one would be to keep the default indent_size = 2 for Markdown files and ignore them when checking for EditorConfig violations.
| [*.patch{,.no,.yet}] | ||
| trim_trailing_whitespace = false | ||
| insert_final_newline = false | ||
| # There are files with different kinds of indentations |
There was a problem hiding this comment.
Another way to address that to keep some indentation properties set, which are suitable for most of the patches and ignoring these files when checking for EditorConfig violations.
| AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
| LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
| SOFTWARE. No newline at end of file |
There was a problem hiding this comment.
Here I also added only a final newline. I don't know why GitHub does not show the "⛔" symbol as in the other files, that here was no final newline before.
|
@BaumiCoder It's great that you made It would be great to be able to add to |
|
I'm using it on https://github.com/jeanp413/open-remote-ssh... It shouldn't change binary files like Edit: I will open some issues on you repo ;) |
I plan to make it available in further ways. See for example issue 13. Nice to hear that this would be really helpful. |
|
Sorry, I will look at your PR after I've released a new version of VSCodium. |
2 participants
This Pull Request is part of an academic research project. More preciously it is for my master's thesis, more background information here.
Definition: EditorConfig violations
The goal of this Pull Request is to address all EditorConfig violations in the project. With EditorConfig you have set for example
insert_final_newline = truefor most files, but there were files without final newlines. I call this an EditorConfig violation as a file content violates the respective EditorConfig property. They can have different kind of impacts. For example trailing spaces, which get remove with the next save, may make the next diff / Pull Request to this file less readable. However, missing final newline may lead to problems when reading the file with some application.How to address them?
To check for such violations and fix them I developed the CLI tool ecformat. However, there are different ways to address the violations:
trim_trailing_whitespace = falsefor them, which avoids automatic removal of such spaces.insert_final_newline = trueto insert the final newlines. To do soecformat fixcan be helpful.ecformat checkfinds. I added a.ecformat_ignorefile for this. It has the common ignore syntax such as a.gitignore. To use such ignore files, provide the name of them with an CLI switch,ecformat check --ignore-file .ecformat_ignore. (The.gitignorefiles are automatically considered inside a git repository.) There are different reasons to ignore files:ecformatas it currently (version 0.2.0) has no support to ignore only a single property for some files.Changes in this Pull Request
For this Pull Request, I addressed all violations in the project in the way, which looks the most correct one to me. (I focused on the Properties of the current EditorConfig specification 0.17.2.) If I fixed file contents, I manually reviewed all of them, making adjustments in the
ecformat fixchanges were necessary.If I picked the wrong way to address the violations for some files, please let me know with a respective Review comment.
Then I will address it in the correct way. I add Review comments myself for special parts, which are especially worthy of discussion. To support the Review of this Pull Request, I tried to group the changes in meaningful commits with respective commit messages. Therefore, may take a look at the commit history.
Avoid EditorConfig violations in the future
Such EditorConfig violations can creep in again. Possible reasons can be that the editor of some contributors has no EditorConfig support, or it would be required to install a respective plugin for the support.
If you want to avoid them in the future, I would suggest integrating a respective utility. For example as part of the CI.
One possibility is to use
ecformatwithecformat check --ignore-file .ecformat_ignore. Another one is editorconfig-checker, which is a more sophisticated tool without the possibility to fix violations.It provides more configuration options, such as ignoring violations in single lines with marker comments.
Let me know if you are interested using one of them. I could add one in this Pull Request or a follow-up Pull Request. If you not consider to use
ecformatin your project, the.ecformat_ignorefile is maybe not worth to merge into the master branch. I can remove it if desired.