Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable lints for some lines and re-enable afterwards or generally reconfigure during check-time #21

Open
NobbZ opened this issue Nov 22, 2018 · 6 comments
Labels
enhancement new feature requested

Comments

@NobbZ
Copy link

NobbZ commented Nov 22, 2018

I have some comments with a link in it and the linter complains about their length.

I can not break the links into multiple lines, as that would destroy them and make dealing with them unnecessarily hard.

On the other hand side, I do not want to disable fill-column checks for the affected files.

Instead I'd like to disable the lint for a section of the file, like this:

;; (elisp-lint-disable 'fill-column)
;; Lets assume this line is to long, but we accept it
;; (elisp-lint-enable 'fill-column)

Maybe we can even override and reconstruct settings?

;; (elisp-lint-push 'fill-column 120)
;; For this line the fill column is 120
;; (elisp-lint-pop 'fill-column)
;; fill coumn is back to its original value now
@gonewest818 gonewest818 added the enhancement new feature requested label Feb 24, 2020
@gonewest818
Copy link
Owner

gonewest818 commented Mar 22, 2020

I'm not sure I can guarantee a case where a variable like fill-column has a value that changes for certain ranges of lines. That would seem to require modifying the internals of the individual checks, and I'd prefer to stay away from that.

Rather, what I think I can support is disabling and enabling checks over certain line number ranges:

;; elisp-lint-disable: fill-column

    ;; For these lines, don't generate fill-column warnings regardless of the line length

    ;; elisp-lint-disable: checkdoc
        ;; Nesting these ranges seems feasible also. 
    ;; elisp-lint-enable: checkdoc

;; elisp-lint-enable: fill-column

Would need to implement a preprocessing scan to walk the file and read these specially formatted comments, and build the list of line number ranges. After the checkers run, we just filter the resulting list and drop warnings that occur in those disabled regions.

I just opened a similar ticket specifically for the package summary line at the top of a file. That's one I can easily special-case because the fill-column test is rather simple (and where I can't use a "pragma" to "wrap" the lines in question, because the package summary is expected to be precisely the first line in the file).

@wpcarro
Copy link

wpcarro commented Sep 1, 2020

Something like ;; elisp-lint-disable-next-line: fill-column might be useful. For my particular use-case, I pretty much need to ignore URLs that I embed in comments that run over 80 characters.

@noctuid
Copy link

noctuid commented Apr 17, 2022

This would be amazing. I've wanted to set up automatic linting that will automatically fail a CI build for a long time, but it's just not possible for elisp because of all the false positives.

@gonewest818
Copy link
Owner

What would you all say to a feature that disables fill-column on comment lines only?

A different kind of special case but seems to be the most common exception.

@noctuid
Copy link

noctuid commented Apr 19, 2022

Package-lint is the real issue for me. I have dozens of invalid warnings in some of my packages. It makes package-lint nearly useless because it's too time-consuming to go through so many warnings every time I make a change to see if there's something new. Checkdoc also has some annoying cases, but you can often work around them. Eventually having some sort of generic mechanism for all validators would be great.

Some linters for other languages will ignore long lines if they contain a url. Not sure how difficult a more specific check like that would be to implement, but an option to just disable fill-column for comment lines would still be useful.

@TLATER
Copy link

TLATER commented Apr 19, 2022

Forgot to check for other PRs before firing that off, but I think that #29 basically covers 99% of the cases in which I'd need to turn off the fill-column part of the linter. I don't currently have other use cases.

A feature to bulk-disable fill-column checks specifically on comments seems like an anti-feature to me. fill-column is most relevant for comments in general, since that's bulk text people actually have to read, which is when reading comfort and disabilities become especially significant. It's only a problem when we try to embed text into comments that isn't intended to be read, which elisp-lint already special cases for nicely.

Just needs that small enhancement for the far too common use case of dumping links to the root cause of some odd edge case I'm working around ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new feature requested
Projects
None yet
Development

No branches or pull requests

5 participants