build: Fix whitespace and add .editorconfig #5674
Merged
Conversation
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
Commands used to search and replace:
$ git grep -Ilz '.' | xargs -0 -I '{}' sh -c \
"printf '%s\n' \"\$(cat '{}')\" >'{}'"
The above commands ensure that there is exaclty 1 line terminator at EOF
(rather than 0 or more than 1) on all non-empty text files.
This fixes all of the "new blank line at EOF" errors raised by git:
$ git diff --check 4b825dc642cb6eb9a060e54bf8d69288fbee4904..HEAD |
grep '^[^+]' | cut -f 3 -d : | LC_ALL=C sort | uniq -c
21 new blank line at EOF.
72 space before tab in indent.
4 trailing whitespace.
Commands used to search and replace:
$ git grep -Ilz '[[:blank:]]$' |
xargs -0 -I '{}' sh -c "printf '%s\n' \"\$(sed -E \
's/[[:blank:]]+$//' '{}')\" >'{}'"
This fixes all of the "trailing whitespace" errors raised by git:
$ git diff --check 4b825dc642cb6eb9a060e54bf8d69288fbee4904..HEAD |
grep '^[^+]' | cut -f 3 -d : | LC_ALL=C sort | uniq -c
72 space before tab in indent.
4 trailing whitespace.
This should make it easier to avoid whitespace errors, as long as the editor used supports it (either natively or through a plugin). See the editorconfig website for the editors that support it: * https://editorconfig.org Note: All text files appear to already be using LF and UTF-8 (or ASCII): $ git ls-files --eol | grep -v -e '^i/lf w/lf' \ -e 'i/none w/none' -e 'i/-text w/-text' i/ w/ attr/text=auto eol=lf ci/check/profiles/sort.py $ git ls-files -z | xargs -0 file -i -h | sed 's/[^:]*: *//' | grep -v -e 'charset=binary' -e 'charset=us-ascii' | LC_ALL=C sort | uniq -c 1 text/html; charset=utf-8 2 text/plain; charset=utf-8 1 text/x-c; charset=utf-8
Git currently correctly detects them as binary; the changes are done to
avoid depending on the auto-detection and also for documentation.
Commands used to list all of the files that git detects as non-text
files:
$ git ls-files --eol | grep -e 'i/-text' -e 'w/-text'
i/-text w/-text attr/text=auto eol=lf etc-fixes/seccomp-join-bug/eecf35c-backports.zip
i/-text w/-text attr/text=auto eol=lf test/appimage/Leafpad-0.8.17-x86_64.AppImage
i/-text w/-text attr/text=auto eol=lf test/appimage/Leafpad-0.8.18.1.glibc2.4-x86_64.AppImage
i/-text w/-text attr/text=auto eol=lf test/filters/memwrexe
i/-text w/-text attr/text=auto eol=lf test/filters/memwrexe-32
i/-text w/-text attr/text=auto eol=lf test/filters/namespaces
i/-text w/-text attr/text=auto eol=lf test/filters/namespaces-32
Note: The committed seccomp filters do not have a file extension, so
ignore them for now.
This fixes all of the "space before tab in indent" errors raised by git:
$ git diff --check 4b825dc642cb6eb9a060e54bf8d69288fbee4904..HEAD |
grep '^[^+]' | cut -f 3 -d : | LC_ALL=C sort | uniq -c
72 space before tab in indent.
Commands used to find the errors:
$ git diff --check 4b825dc642cb6eb9a060e54bf8d69288fbee4904..HEAD
$ git grep -In "$(printf '\t') "
Note: Unlike "space before tab in indent", the reverse ("space after tab
in indent") is not reported by git. That is because spaces could be
intentionally used for alignment or line continuation, but in some cases
they are being used for indentation together with tabs and in others the
formatting is misaligned. The second command was used to help find and
fix these other issues.
And the surrounding paragraphs. Relates to netblue30#2784.
This appears to be the only C file in the repository that uses spaces
for indentation.
Commands used to check for the above:
$ git grep '^ ' -- '*.c' '*.h'
Commands used to search and replace:
$ f=test/filters/namespaces.c; printf '%s\n' \
"$(sed 's/ /\t/g' "$f")" >"$f"
Note: The mmap call was aligned manually.
Added on commit 5116c1c ("testing", 2022-12-24).
To match the common usage; see for example src/firejail/firejail.h. Added on commit 960b4da ("add tool to dump seccomp filters", 2020-02-17).
Command used to find the errors:
$ git grep -I '^ [^*]' -- test/
Misc: All of the affected files were added in 2016.
Almost all of the shell scripts in the repository use tabs for
indentation (or have no indentation at all):
$ git grep -Il '^\t' -- '*.sh' | wc -l
19
$ git grep -Il '^ ' -- '*.sh' | wc -l
5
$ git grep -IL '^[ \t]' -- '*.sh' | wc -l
25
So do the same in the few shell scripts that currently use spaces for
indentation.
Except for the following file:
* platform/rpm/mkrpm.sh
Not sure if it's following a packaging-specific scheme, so just fix the
one indentation inconsistency in it and otherwise leave it as is for
now.
Command used to search for shell scripts using spaces for indentation:
$ git grep -In '^ ' -- '*.sh'
Changes:
* Fix spaces being used for indentation in some lines in C
* Remove leading spaces before some goto labels
* Remove leading spaces before the start of some multiline comments
* Change leading spaces to tabs in some multiline macros
* Add missing asterisk to some multiline comments (to match other
multiline comments and because they are false positives in the
commands below)
Note: Leading spaces can be used for alignment (such as in function
parameters and function arguments in C) and for line continuation (such
as in long commands in shell scripts). However, in the above changes
the leading spaces are used for other reasons and do not seem to fit
with the style used.
Commands used to search for errors:
$ git grep -In '^ [^*]' | grep -E -v \
-e '(COPYING|README|RELNOTES|configure(.ac)?):' \
-e '^[^:]+.(md|yml|py):' -e '(bash|zsh)_completion/' \
-e '^contrib/syntax/' -e '^etc/templates/.*\.txt:' -e '^m4/' \
-e '^platform/debian/' -e '^src/man/.*\.txt:' \
-e '.*mkrpm.sh:' -e '.*extract_errnos.sh:'
Commands used to list the file extensions used in the project:
$ git ls-files | sed -En 's/.*(\.[^.]+)$/\1/p' |
LC_ALL=C sort | uniq -c
For rules that are more specific to a given directory, put a dedicated
.editorconfig file in it.
|
By the way, I think that in vim it would be something like See also the
Thanks! |
|
all merged! |
rusty-snake
reviewed
Feb 25, 2023
kmk3
added a commit
that referenced
this pull request
Feb 25, 2023
As pointed out by @rusty-snake[1]: > I think this is intentional to test if firejail can parse commands > with leading spaces. This amends commit b406b24 ("tests: Fix mixed space/tabs indentation", 2023-02-19) / PR #5674. Note: This is the only profile in test/ that the commit changed: $ git show --pretty= --name-only b406b24 -- test/ test/fs/private-whitelist.exp test/network/firemon-route.exp test/profiles/test2.profile [1] #5674 (comment)
kmk3
added a commit
that referenced
this pull request
Feb 25, 2023
This partially reverts commit 3754680 ("docs: remove indents on top-level lists and tables", 2023-02-01) from PR #5674. Commands used to undo the changes: $ f=.github/pull_request_template.md; \ git show 3754680~1:"$f" >"$f" I had assumed that a blank line after a list item would end the list (and so I was confused by the amount of indentation used), but that is apparently not the case. See the file rendered before/after the commit[1] [2]. Relates to #2784. Reported by @rusty-snake[3]. [1] https://github.com/netblue30/firejail/blob/f5d8d8cc7af8f8816c47623515babcefceb7e22f/.github/pull_request_template.md [2] https://github.com/netblue30/firejail/blob/37546800876d977d77cc86d9b307f8cfa714c1dd/.github/pull_request_template.md [3] #5674 (comment)
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Mar 26, 2023
This amends commit 3754680 ("docs: remove indents on top-level lists and tables", 2023-02-01) / PR netblue30#5674. Relates to netblue30#2784.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Using .editorconfig should make it easier to avoid whitespace errors, as long
as the editor used supports it (either natively or through a plugin).
See the editorconfig website for the editors that support it:
Relates to #4572 #4575.
Cc: @a1346054 @reinerh (from #4572 #4575)