mrtazz / checkmake

experimental linter/analyzer for Makefiles
MIT License
1.02k stars 44 forks source link

Please do not ouptut CR sign. #54

Closed Kamilcuk closed 2 years ago

Kamilcuk commented 2 years ago

Expected behaviour

Weeell, do not output CR on Linux.

Actual behaviour

Weeell, outputs CR character on Linux.

$ LC_ALL=C checkmake --format={{.LineNumber}}:{{.Rule}}:{{.Violation}} /tmp/Makefile | head -n1 | hexdump -C
00000000  30 3a 6d 69 6e 70 68 6f  6e 79 3a 4d 69 73 73 69  |0:minphony:Missi|
00000010  6e 67 20 72 65 71 75 69  72 65 64 20 70 68 6f 6e  |ng required phon|
00000020  79 20 74 61 72 67 65 74  20 22 61 6c 6c 22 0d 0a  |y target "all"..|
#                                                    ^^ HERE

Output of checkmake --version

Compiled on archlinux today, installed from AUR.

$ checkmake --version
checkmake 0.1.0-47-g575315c built at 2021-09-29T09:02:23Z by Kamil Cukrowski <kamilcukrowski@gmail.com> with go version go1.17.1 linux/amd64

Output of checkmake --debug <your makefile>

$ LC_ALL=C checkmake --format={{.LineNumber}}:{{.Rule}}:{{.Violation}} --debug /tmp/Makefile 
2021/09/29 12:07:30 Unable to match line '' to a Rule or Variable
2021/09/29 12:07:30 Unable to parse config file "checkmake.ini", running with defaults
2021/09/29 12:07:30 Running rule 'maxbodylength'...
2021/09/29 12:07:30 iniFile not initialized
2021/09/29 12:07:30 Running rule 'minphony'...
2021/09/29 12:07:30 iniFile not initialized
2021/09/29 12:07:30 Running rule 'phonydeclared'...
2021/09/29 12:07:30 iniFile not initialized
2021/09/29 12:07:30 Running rule 'timestampexpanded'...
2021/09/29 12:07:30 iniFile not initialized
0:minphony:Missing required phony target "all"
0:minphony:Missing required phony target "clean"
0:minphony:Missing required phony target "test"
1:phonydeclared:Target "all" should be declared PHONY.

Output of make --version

$ LC_ALL=C make --version
GNU Make 4.3
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Sample Makefile to reproduce issue

all:

So I was trying to integrate checkmake with https://github.com/iamcco/coc-diagnostic, spend good 2 hours figuring why it does not work. Seems like coc-diagnostic does not like CR characters (which I will tr -d '\'r fix on my config side) but I believe *unix tools shouldn't output CR characters anyway. I tried reading the source code, but I know nothing about go, so I do not know where to fix.

Thanks for this amazing project.

Lenbok commented 1 year ago

So, #55 removed both the \r and the \n, whereas the bug report just wanted the \r gone. OK then, so let's try adding a \n in my report format:

$ go run github.com/mrtazz/checkmake/cmd/checkmake@latest --format='{{.FileName}}:{{.LineNumber}}:{{.Rule}}:{{.Violation}}\n' Makefile
Makefile:7:maxbodylength:Target body for "help" exceeds allowed length of 5 (6).\nMakefile:127:minphony:Missing required phony target "all"\nMakefile:127:minphony:Missing required phony target "test"\nexit status 3

The \n is being output literally.