[2/2] Improve check-whitespace output

Commit Message

Chris Webster Dec. 16, 2022, 8:32 a.m. UTC

From: "Chris. Webster" <[e-mail]>

A message in the step log will refer to the Summary output.

The job summary output now has links to the commits and files.

Signed-off-by: Chris. Webster <[e-mail]>
---
 .github/workflows/check-whitespace.yml | 34 +++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)

Comments

Junio C Hamano Dec. 16, 2022, 10:13 a.m. UTC | #1

"Chris. Webster via GitGitGadget" <[e-mail]> writes:

> From: "Chris. Webster" <[e-mail]>

> Subject: Re: [PATCH 2/2] Improve check-whitespace output

The same comment about specificity of the improvements applies to
this one, too.  Also, I forgot to point out that our usual commit
title takes the form of "<area>: <description>", e.g.

	Subject: [PATCH 2/2] ci: show $X in check.whitespace output

where "show $X" is meant to be a more concrete phrase than "improve"
what the change is about and how it improves the output.

> +          echo "Run these commands to correct the problem:" >>$GITHUB_STEP_SUMMARY
> +          echo "1. \`git rebase --whitespace=fix ${goodparent}\`" >>$GITHUB_STEP_SUMMARY
> +          echo "1. \`git push --force\`" >>$GITHUB_STEP_SUMMARY

It's a bit curious to see two "1." and not "1." followed by "2."
here.  Is this meant to be processed by markdown or something so we
do not have to do the numbering ourselves, or something?

> +          echo " " >>$GITHUB_STEP_SUMMARY
> +          echo "Errors:" >>$GITHUB_STEP_SUMMARY
>            for i in "${problems[@]}"
>            do
>              echo "${i}" >>$GITHUB_STEP_SUMMARY

Thanks.

Chris Webster Dec. 20, 2022, 12:33 a.m. UTC | #2

On Fri, Dec 16, 2022 at 2:13 AM Junio C Hamano <[e-mail]> wrote:
> It's a bit curious to see two "1." and not "1." followed by "2."
> here.  Is this meant to be processed by markdown or something so we
> do not have to do the numbering ourselves, or something?

Yes, GITHUB_STEP_SUMMARY is treated as markdown.

Thanks for the feedback,
...chris.

Patch

diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index 3a99073bc33..da557fd5914 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -20,46 +20,62 @@  jobs:
     - name: git log --check
       id: check_out
       run: |
+        baseSha=${{github.event.pull_request.base.sha}}
         problems=()
         commit=
         commitText=
-        lastcommit=
+        commitTextmd=
+        goodparent=
         while read dash sha etc
         do
           case "${dash}" in
           "---")
             if test -z "${commit}"
             then
-              lastcommit=${sha}
+              goodparent=${sha}
             fi
             commit="${sha}"
             commitText="${sha} ${etc}"
+            commitTextmd="[${sha}](https://github.com/${{ github.repository }}/commit/${sha}) ${etc}"
             ;;
           "")
             ;;
           *)
             if test -n "${commit}"
             then
-              problems+=("" "--- ${commitText}")
+              problems+=("1) --- ${commitTextmd}")
               echo ""
               echo "--- ${commitText}"
               commit=
             fi
-            problems+=("${dash} ${sha} ${etc}")
-            echo "${problems[-1]}"
+            case "${dash}" in
+            *:[1-9]*:) # contains file and line number information
+              dashend=${dash#*:}
+              problems+=("[${dash}](https://github.com/${{ github.repository }}/blob/${{github.event.pull_request.head.ref}}/${dash%%:*}#L${dashend%:}) ${sha} ${etc}")
+              ;;
+            *)
+              problems+=("\`${dash} ${sha} ${etc}\`")
+              ;;
+            esac
+            echo "${dash} ${sha} ${etc}"
             ;;
           esac
-        done <<< $(git log --check --pretty=format:"---% h% s" ${{github.event.pull_request.base.sha}}..)
+        done <<< $(git log --check --pretty=format:"---% h% s" ${baseSha}..)
 
         if test ${#problems[*]} -gt 0
         then
           if test -z "${commit}"
           then
-            lastcommit=${{github.event.pull_request.base.sha}}
+            goodparent=${baseSha: 0:7}
           fi
-          echo "A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY
+          echo "