Better EditorConfig Settings & Code Styles Validation via Travis CI #6523
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,79 @@ | ||
| root = true | ||
|
|
||
|
|
||
| ## Repository Configurations | ||
| ############################ | ||
| [.{git*,editorconfig,*.yml}] | ||
| charset = utf-8 | ||
| end_of_line = lf | ||
| indent_style = space | ||
| indent_size = unset | ||
| trim_trailing_whitespace = true | ||
| insert_final_newline = true | ||
|
|
||
| [.gitmodules] | ||
| indent_style = tab | ||
|
|
||
|
|
||
| ## Shell Scripts | ||
| ################ | ||
| [*.sh] | ||
| charset = utf-8 | ||
| end_of_line = lf | ||
| indent_style = tab | ||
| indent_size = unset | ||
| trim_trailing_whitespace = true | ||
| insert_final_newline = true | ||
|
|
||
|
|
||
| ## Make Files | ||
| ############# | ||
| [Makefile] | ||
| charset = utf-8 | ||
| end_of_line = lf | ||
| indent_style = tab | ||
| indent_size = 2 | ||
| trim_trailing_whitespace = true | ||
| insert_final_newline = true | ||
|
|
||
|
|
||
| ## Haskell Sources | ||
| ################## | ||
| [*.hs] | ||
| charset = utf-8 | ||
| end_of_line = unset | ||
| indent_style = space | ||
| indent_size = unset | ||
|
There was a problem hiding this comment. The general coding style uses two spaces, but Haskell indentation is weird sometimes. Does verification fail if we set this? There was a problem hiding this comment. Yes it would fail because the validator tool expects all indentation to be a multiple of the base value. Unfortunately the EditorConfig project doesn't offer separate settings for editor/IDEs configurations and validators right now. Since most coders add extra spaces to obtain elegant alignments on code that spans multiple lines, it often becomes necessary to either unset As for markdown (and other lightweight markup syntaxes) the problem affect code blocks, which could be using even or odd indentation, depending on the language snippet being included. Hopefully in future editions of EditorConfig we'll see new settings to address validation aspects only. Right now, one is forced to compromise on some features to obtain both editor settings and CI validation — e.g. EditorConfig doesn't (and will never) support I'll take a further look at EClint's documentation, there are some ways to customize validation settings and overriding the There was a problem hiding this comment. I've checked EClint documentation, and there's no easy way to override |
||
| trim_trailing_whitespace = true | ||
| insert_final_newline = true | ||
|
|
||
|
|
||
| ## Markdown | ||
| ########### | ||
| [*.{markdown,md}] | ||
| charset = utf-8 | ||
| end_of_line = unset | ||
| indent_style = space | ||
| indent_size = unset | ||
| trim_trailing_whitespace = false | ||
|
There was a problem hiding this comment. I think we're not using trailing spaces anywhere, might be ok to set this to true. There was a problem hiding this comment. I personally never use trailing spaces for hard line-breaks, but we need to check with @jgm for this setting, after all GFM does support that feature so potentially it could be used in the GFM documentation or templates, and if I remember correctly pandoc also supports the feature via extensions. I always set There was a problem hiding this comment. I'm sure there are tests that include trailing whitespace, even if we don't use it in documentation (and for all I recall, we might). There was a problem hiding this comment. I've revised the rules for
I've checked. the only markdown file with trailing spaces is So, @jgm, should we set the rules for markdown files outside the Unless we need to use them in the documentation, it would prevent unclean commits. |
||
| insert_final_newline = true | ||
|
|
||
|
|
||
| ## Lua and Perl | ||
| ############### | ||
| [*.{lua,pl}] | ||
| charset = utf-8 | ||
| end_of_line = unset | ||
| indent_style = space | ||
| indent_size = 2 | ||
| trim_trailing_whitespace = true | ||
| insert_final_newline = true | ||
|
|
||
|
|
||
| ## Test Files | ||
| ############# | ||
| [test/**] | ||
|
There was a problem hiding this comment. I just realized that we still want the general rules to apply for Haskell and Lua files in There was a problem hiding this comment. Maybe we should apply the above pattern only to markdown files? After all, I believe that is only the markdown files that need relaxed rules to allow testing edge-cases and malformed documents. |
||
| indent_style = unset | ||
| indent_size = unset | ||
| insert_final_newline = unset | ||
| trim_trailing_whitespace = unset | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,16 @@ | ||
| * text=auto | ||
|
|
||
| ## Repository Configuration | ||
| ########################### | ||
| .*.yml text eol=lf | ||
| .editorconfig text eol=lf | ||
| .gitattributes text eol=lf | ||
| .gitconfig text eol=lf | ||
| .gitignore text eol=lf | ||
| .gitmodules text eol=lf | ||
|
|
||
|
|
||
| *.sh text eol=lf | ||
| Makefile text eol=lf | ||
|
|
||
| test/fb2/reader/* -text |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| dist: trusty | ||
|
There was a problem hiding this comment. If I understand correctly, then this is a temporary solution and would be converted into a GitHub Action workflow after merging? There was a problem hiding this comment. I leave that choice to you, as you're surely more qualified to decide on this. Personally, I always use Travis for code validation, to avoid bumping into the free GH Actions limits (since validation is done for every commit on every branch, having many repositories could quickly reach the limit). Another reason to keep it on Travis (instead of Circle or GH Actions) might be performance, since it would run in a parallel job. There was a problem hiding this comment. I'd like to avoid using multiple CI providers; if we can do it all in GH actions that seems better to me. There was a problem hiding this comment. There's a ready availabe EClint action: https://github.com/marketplace/actions/eclint I haven't use GH Actions before, some I'm not sure how to set it up. Can someone else add the action? |
||
|
|
||
| git: | ||
| depth: false | ||
|
|
||
| install: | ||
| - npm install -g eclint | ||
|
|
||
| script: | ||
| # ============================================== | ||
| # EditorConfig Code Styles Validation via EClint | ||
| # ============================================== | ||
| # https://editorconfig.org | ||
| # https://www.npmjs.com/package/eclint | ||
| - bash ./validate.sh | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -215,20 +215,20 @@ return { | |
|
|
||
| group 'walk_block' { | ||
| test('block walking order', function () | ||
| local acc = {} | ||
|
There was a problem hiding this comment. These should be indented by two additional spaces. There was a problem hiding this comment. Commit 5bdf167 should fix this (hopefully). |
||
| local nested_nums = pandoc.Div { | ||
| pandoc.Para{pandoc.Str'1'}, | ||
| pandoc.Div{ | ||
| pandoc.Para{pandoc.Str'2'}, | ||
| pandoc.Para{pandoc.Str'3'} | ||
| }, | ||
| pandoc.Para{pandoc.Str'4'} | ||
| } | ||
| pandoc.walk_block( | ||
| nested_nums, | ||
| {Para = function (p) table.insert(acc, p.content[1].text) end} | ||
| ) | ||
| assert.are_equal('1234', table.concat(acc)) | ||
| end) | ||
| }, | ||
|
|
||
|
|
||
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 we are using 4 spaces, at least that's what I've been using. But I might be wrong and I am ok with either.
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.
To the best of my knowledge, shell scripts should use tabs only for indentation, because spaces indents could lead to problems in some contexts — the matter is subject of dispute (as everything relating to the tabs-vs-space flamewars) but this seems to be the prevalent opinion, at least on long StackOverflow discussions.
But I'll set it to any value you suggest, the most important thing is to keep the code consistent.
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.
The reason why tabs are preferable is in case shell scripts are used to dynamically generate other scripts via pipes and redirections, where tabs are a safer option because common used tools will auto-strip leading tabs (but not spaces).
An example:
https://www.howtobuildsoftware.com/index.php/how-do/ctWc/shell-sh-perforce-error-in-creating-p4-label-non-interactively-as-part-of-shell-script
Beside that, it boils down to a matter of personal choice (and the many flamewars on the subject didn't really bring any solution to the issue). I personally prefer using spaces in version controlled projects, for they create more readable diffs. But since in Make files tabs are mandatory, and shell scripts are sometimes used to manipulate Make files, I prefer to adopt tabs.
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.
Some interesting trivia on the subject:
So, yes, I'm for spaces-indentation, but make an exception for shell scripts.
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.
Interesting, I didn't know. Thanks!