KEMBAR78
fix: Improve max_line_length rule with comment behavior by milouse · Pull Request #67 · coffeelint/coffeelint · GitHub
Skip to content

fix: Improve max_line_length rule with comment behavior #67

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 11, 2020
Merged

fix: Improve max_line_length rule with comment behavior #67

merged 3 commits into from
Jun 11, 2020

Conversation

milouse
Copy link
Contributor

@milouse milouse commented Jun 11, 2020

The current max_line_length will report annoying error on this kind of comment:

class Test
  # Build ajax request with some default options.
  #
  # @private
  #
  # @param opts [Object] Fetch API init object with some very long comment line olalala
  # @see https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/fetch fetch API on mozdev
  _request: (opts) ->
    doSomething

With the current implementation, coffeelint will report an error on both line 6 (@param opts…) and 7 (@see…).

This rule should, IMHO only report line 6 with "limitComments": true and nothing with "limitComments": false.

This MR fix the two inner regexps to follow the spec above.

@UziTech UziTech added the needs tests PR Needs tests in order to merge label Jun 11, 2020
@UziTech
Copy link
Member

UziTech commented Jun 11, 2020

Thanks for this. Could you write a test for this in/test/test_max_line_length.coffee?

@milouse milouse force-pushed the master branch 3 times, most recently from 9f395ad to 4e33391 Compare June 11, 2020 16:55
@milouse
Copy link
Contributor Author

milouse commented Jun 11, 2020

I've just add required test. Sorry for the noise, I force-pushed a bit to fix some typos. I hope my tests are well written and sufficient.

@milouse
Copy link
Contributor Author

milouse commented Jun 11, 2020

… And I change my exemple string to let the previous CI test pass again :)

@UziTech UziTech removed the needs tests PR Needs tests in order to merge label Jun 11, 2020
@UziTech UziTech changed the title Improve max_line_length rule with comment behavior fix: Improve max_line_length rule with comment behavior Jun 11, 2020
@UziTech UziTech merged commit aa7930a into coffeelint:master Jun 11, 2020
github-actions bot pushed a commit that referenced this pull request Jun 11, 2020
@semantic-release-bot
chore(release): 3.2.7 [skip ci]
2d9c69c 
## [3.2.7](v3.2.6...v3.2.7) (2020-06-11)

### Bug Fixes

* **deps:** update dependency ignore to ^5.1.8 ([#68](#68)) ([dc316a4](dc316a4))
* Improve max_line_length rule with comment behavior ([#67](#67)) ([aa7930a](aa7930a))
* **deps:** update dependency ignore to ^5.1.8 ([2a6b1d0](2a6b1d0))
@github-actions
Copy link
Contributor

🎉 This PR is included in version 3.2.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@milouse @UziTech