[check] Split github action into two stages (source, output) #4533
Conversation
jensmaurer
force-pushed
the
chk
branch
2 times, most recently
from
b44a4fb
to
ead66e9
Compare
jensmaurer
changed the title
|
|
|
It seems that the source location in the message is still useful. Otherwise, it only appears when you look at it from "Files changed" (see image below) and not in the workflow run logs. |
|
Ah, nice. |
|
Excellent, thanks! |
There was a problem hiding this comment.
- I don't understand all the detailed
sed/grepmagic, but please do what you think is best. - The split is great. I might maybe call the two phases
check-sourceandcheck-output? - Regarding the commit message: this isn't just "CI", since it's also precommit. I recommend phrasing the commit message as "Split GitHub action into two stages (source, output)".
tools/check.sh
Outdated
|
|
||
| # For some reason, the file/line/col information is not shown in the GUI. | ||
| # Make sure to leave it in the message proper. | ||
| ! sed 's/^\(.\+\.tex\):\([0-9]\+\):/::error file=source\/\1,line=\2::\1:\2:'"$*"': /; s/^\([^:][^:]\)/::error ::'"$*"': \1/' | |
There was a problem hiding this comment.
I think the last sed command will be ill-formed if the $* message contains a / e.g. something like | fail "misplaced begin/end"
It's probably OK, but if we want to allow / in the errors we could use another character less likely to be used in the error string, e.g. @
! sed 's@^\(.\+\.tex\):\([0-9]\+\):@::error file=source/\1,line=\2::\1:\2:'"$*"': @; s@^\([^:][^:]\)@::error ::'"$*"': \1@' |
(N.B. this also allows source/ instead of source\/)
There was a problem hiding this comment.
The trouble is that awk's regex machinery doesn't seem to allow references to sub-matches grouped by ().
But using @ as the sed separator is probably a good compromise.
There was a problem hiding this comment.
GNU awk's gensub() function supports back-refs.
There was a problem hiding this comment.
Great, but @burblebee already dislikes GNU-isms in sed, so better avoid them for such a central functionality.
Regarding that. For my recent commits that did not touch the LaTeX sources, I took the liberty to use https://www.conventionalcommits.org/. |
Also improve the visibility of error messages.
|
@tkoeppe, I've addressed your concerns. Please have another look. |
jensmaurer
changed the title
|
@jensmaurer: Great, thanks, ship it! @JohelEGP: Yeah, I didn't have a good regular label at hand previously, so I just sayd "Action" a lot, but I think Jens has come up with a nice one that we can use from now on. |
Also improve the visibility of error messages.
Resolves #4529