golines breaks lines with //nolint
guettli opened this issue · 3 comments
Original line:
deviceID, err := strconv.ParseInt(strings.TrimPrefix(node.Spec.ProviderID, providerPrefix), 10, 32) //nolint:revive // https://www.reddit.com/r/golang/comments/10mi0el/strconvparseintmyid_10_32_avoid_magic_numbers/
after running golines
:
deviceID, err := strconv.ParseInt(
strings.TrimPrefix(node.Spec.ProviderID, providerPrefix),
10,
32,
) //nolint:revive // https://www.reddit.com/r/golang/comments/10mi0el/strconvparseintmyid_10_32_avoid_magic_numbers/
I like the output of golines.
But now I get warnings from the linter:
foo/bar.go:55:3: add-constant: avoid magic numbers like '10', create a named constant for it (revive)
10,
^
foo/bar.go:56:3: add-constant: avoid magic numbers like '32', create a named constant for it (revive)
32,
I think it would be great if golines would skip lines containing "//nolint" by default.
What do you think?
I can see an argument for skipping lines that contain //nolint
special comments. That is, I can imagine cases where just a //nolint
comment causes an undesired break. I'm not sure whether I think the change is justified or not, but it's worth considering.
But in this case, I think there's a much simpler fix than patching golines
: you can simply remove the (very long) second comment with the Reddit URL. That would probably leave you with a line short enough that golines
would ignore it.
@telemachus is there already a way to configure golines
, so that lines containing a special string get ignored?
If not, would you accept a patch?
@telemachus is there already a way to configure
golines
, so that lines containing a special string get ignored?
I don't think so.
If not, would you accept a patch?
I'm not the maintainer, but I can say that a similar request is labelled as an enhancement. That suggests to me that a patch would at least be considered, but I can't be sure.