Add a new flag to control line-length of comment lines separately.

[csilvers]

This allows users to have a longer line-length limit for code than for natural-language text.

One example is a project that wants "most" code to fit in 80 columns, but will allow the occassional long line. The best way to implement this in lll right now is to have a longer line limit (say, 100 columns) with an understanding the code-reviewers will make sure most lines are not over 80 columns. But we still want all comment lines to fit within 80 columns, since there's no good reason for them not to. This new flag allows for that.

[csilvers]

I forgot to say in the commit message, but I also fixed a buglet in util_test where expected and actual were reversed.

[Zamiell]

@walle Any chance of merging this? It's a little ridiculous that the lll linter complains about code like the following:

// Get the project path
// https://stackoverflow.com/questions/18537257/how-to-get-the-directory-of-the-currently-running-file
if v, err := os.Executable(); err != nil {
    fmt.Println("Failed to get the path of the currently running executable:", err)
} else {
    projectPath = filepath.Dir(v)
}

[walle]

@Zamiell Thanks for bumping this PR! Somehow I completly missed it :(

[Zamiell]

No problem, thanks for your work on this excellent linter.

[Zamiell]

As a side note, is it possible to just make lll ignore lines that are over X characters when there are no spaces in them? Either by default or possibly behind a new flag? (And maybe it should only apply to comments.)

[csilvers]

Thanks for the detailed feedback!

We wanted to use lll in the context of golangci-lint. It turns out they have their own implementation of lll so we ended up making changes there. golangci-lint gives access to the AST, so we implemented things that way, which was more annoying in some ways but made it easier to, e.g., deal with multi-line comments.

The end result ended up being quite complicated, and it's still not super-well tested within our org, but I'm happy to share it if you'd like! But these days I'm basically on childcare full time and won't be able to get to this PR for a while. Someone else who could use it is very welcome to commandeer it and push it over the finish line!

[codecov-io]

Codecov Report

Merging #10 into master will increase coverage by 0.40%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
+ Coverage   90.76%   91.17%   +0.40%     
==========================================
  Files           2        2              
  Lines          65       68       +3     
==========================================
+ Hits           59       62       +3     
  Misses          4        4              
  Partials        2        2              

Impacted Files	Coverage Δ
lll.go	89.47% <100.00%> (ø)
utils.go	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4438bcc...8c79f08. Read the comment docs.

[walle]

@Zamiell l I would add a new flag for that. Let me see if I can get some time to add the feature soon :)

@csilvers Aha ok, I didn't know it was re-implemented in golangci-lint I'll take a look.

And now worries about getting the PR over the finish line, I will try to dedicate some time to it asap. And maybe add some more things :)

Thank you for your contribution!

[Zamiell]

@walle Thanks. To go a little bit more in depth on my suggestion, it is probably better to just copy the various flags (and the corresponding algorithms) from the most popular long-line-linter in the world, eslint max-len: https://eslint.org/docs/rules/max-len

We can see the source code for it here: https://github.com/eslint/eslint/blob/master/lib/rules/max-len.js

Specifically, relating to my suggestion, it has flags for "ignoreStrings", and "ignoreUrls", both of which are set to true by default. I think having them be true by default is a sensible idea, for both max-len and for lll!

Furthermore, if you have the time, I think it would actually be ideal if lll provided all of the flags that max-len does, as I think that people will probably expect similar functionality.

[Zamiell]

walle, any progress on this? I also have another request, but I'll put it in a new issue.