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.

Sign up for GitHub

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

Jump to bottom

Add a workflow for nitpicking on trailing spaces and similar issues (to discuss) #892

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add a workflow for nitpicking on trailing spaces and similar issues (to discuss) #892

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions .github/workflows/nitpicks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
name: Nitpicks

on:
pull_request:
paths:
- 'DC-*'
- 'xml/*'
- 'adoc/*'

jobs:
nitpicks:
runs-on: ubuntu-latest
strategy:
fail-fast: false
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0
- name: Checking out relevant branches
run: |
git checkout $GITHUB_BASE_REF || { echo "There's a Git issue"; exit 1; }
git checkout $GITHUB_HEAD_REF || { echo "There's a Git issue"; exit 1; }

- name: Checking for tabs
run: |
tab=$(git diff -U0 $GITHUB_BASE_REF..$GITHUB_HEAD_REF | sed -n '/^+/ p' | sed -n '/\t/ p' | sed -r -e 's/\t/→/g')
if [[ -n "$tab" ]]; then
echo -e "\nThis pull request introduces tabs (→) on the following lines:"
echo -e "\n$tab\n"
Comment on lines +29 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to use the GitHub formatted error message to make it visually distinct from other messages:

https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message

Syntax

::error file={name},line={line},col={col}::{message}

Most of these parameters are optional. So you would probably use something like this:

Suggested change
echo -e "\nThis pull request introduces tabs (→) on the following lines:"
echo -e "\n$tab\n"
echo "::error::This pull request introduces tabs (→) on the following lines:"
echo "::error::$tab"

You may want to apply it to the other echo commands as well.

Copy link
Author

Choose a reason for hiding this comment

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

An exit command isn't really necessary if you use the template from previous item. The Action will stop this step anyway.

If that is true, then using two echos with ::error:: in succession as suggested here won't work, because it'd exit after the first ::error:: message. I am also unsure if ::error:: allows for -e which is extremely necessary for the second echo.

Additionally, an exit just blows up the code for my taste.

exit 1; is 7 characters ... if anything "blows up the code", it's hardly that.^^

You can use the continue-on-error: true. What you need is to "summarize" the result of each steps into an output. Then in a last step you check each one and react accordingly

I should probably do something like that. It could have been less work though if continue-on-error worked better though. :/

Copy link
Contributor

Choose a reason for hiding this comment

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

An exit command isn't really necessary if you use the template from previous item. The Action will stop this step anyway.

If that is true, then using two echos with ::error:: in succession as suggested here won't work, because it'd exit after the first ::error:: message. I am also unsure if ::error:: allows for -e which is extremely necessary for the second echo.

Right, maybe I misremembered that.

Additionally, an exit just blows up the code for my taste.

exit 1; is 7 characters ... if anything "blows up the code", it's hardly that.^^

It's not only this line, there are other lines. But it's probably a matter of taste, so ignore that. 😉

I should probably do something like that. It could have been less work though if continue-on-error worked better though. :/

Right. It was just an idea. As you wrote, maybe for the future it would be better anyway to move it into a "real" GH Action. But for the time being, you could leave it as is.

exit 1
else
echo "This looks aight."
exit 0
fi

- name: Checking for Windows/Mac line ends
run: |
lineends=$(git diff -U0 $GITHUB_BASE_REF..$GITHUB_HEAD_REF | sed -n '/^+/ p' | sed -n '/\r/ p' | sed -r -e 's/\r/↲/g')
if [[ -n "$lineends" ]]; then
echo -e "\nThis pull request introduces Windows/Mac line ends (↲) on the following lines:"
echo -e "\n$lineends\n"
exit 1
else
echo "This looks aight."
exit 0
fi

- name: Checking for trailing characters
run: |
trail=$(git diff -U0 $GITHUB_BASE_REF..$GITHUB_HEAD_REF | sed -n '/^+/ p' | sed -n '/[  \t]$/ p' | sed -r -e 's/ *$/•/g' -e 's/\t*$/→/g' -e 's/ *$/⋄/g')
if [[ -n "$trail" ]]; then
echo -e "\nThis pull request introduces trailing spaces (•)/tabs (→)/protected spaces (⋄) on the following lines:"
echo -e "\n$trail\n"
exit 1
else
echo "This looks aight."
exit 0
fi

- name: Checking for long lines
# We exclude screens, there are legitimate reasons for them to be long.
run: |
potential=$(git diff -U1000 $GITHUB_BASE_REF..$GITHUB_HEAD_REF | tr '\n' '\r' | sed -r -e 's,<screen[^>]*/>,,g' -e 's,</screen[^>]*>,⋘,g' -e 's,<screen[^/>]*>[^⋘]*⋘,,g' | tr '\r' '\n' | sed -n '/^+/ p')
len=$(echo -e "$potential" | wc -l)
long=''
for n in $(seq 1 "$len"); do
line=$(echo -e "$potential" | sed -n "$n p")
if [[ $(echo "$line" | wc -c) -gt 91 ]]; then
long+="\n$line"
fi
done
if [[ -n "$long" ]]; then
echo -e "\nThis pull request introduces long lines (80+ characters) in at least the following spots:"
echo -e "$long\n"
exit 1
else
echo "This looks aight."
exit 0
fi