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

Rewrite the max-len rule #528

Closed
dmp42 opened this Issue Jan 16, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@dmp42
Contributor

dmp42 commented Jan 16, 2014

Following on this #525 (comment)

Here is what I would like to do:

  • honor windows and unix line endings (eg: DON'T count \r in the line length if the line ends with \r\n)
  • stop saying that the first line is "line 0" (+1 to all line numbers)
  • rewrite broken stringRepeat

@dmp42 dmp42 referenced this issue Jan 16, 2014

Closed

Fix #528 #529

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Jan 17, 2014

Member

Can you explain what the problem is that you're trying to solve? "Rewrite" is a solution, and issues are supposed to describe problems. :)

Member

nzakas commented Jan 17, 2014

Can you explain what the problem is that you're trying to solve? "Rewrite" is a solution, and issues are supposed to describe problems. :)

@dmp42

This comment has been minimized.

Show comment
Hide comment
@dmp42

dmp42 Jan 17, 2014

Contributor

This is following this comment: #525 (comment) from michaelficarra

As for the problems, as stated:

  • carriage return in line endings are counted
  • reported line number is offset by 1
  • stringRepeat is broken

Best regards.

Contributor

dmp42 commented Jan 17, 2014

This is following this comment: #525 (comment) from michaelficarra

As for the problems, as stated:

  • carriage return in line endings are counted
  • reported line number is offset by 1
  • stringRepeat is broken

Best regards.

@dmp42

This comment has been minimized.

Show comment
Hide comment
@dmp42

dmp42 Jan 17, 2014

Contributor

And this does fix as well the crash when file is empty, as described in #525

Contributor

dmp42 commented Jan 17, 2014

And this does fix as well the crash when file is empty, as described in #525

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Jan 17, 2014

Member

So that means the pull request for this issue also fixes #525? Does the pull request for #525 is no longer valid?

Member

nzakas commented Jan 17, 2014

So that means the pull request for this issue also fixes #525? Does the pull request for #525 is no longer valid?

@dmp42

This comment has been minimized.

Show comment
Hide comment
@dmp42

dmp42 Jan 18, 2014

Contributor

It does fix #525 as well - I guess it boils down to which one you guys want / prefer to merge on which branch, but yes, if this PR gets in I will cancel the other one.

Contributor

dmp42 commented Jan 18, 2014

It does fix #525 as well - I guess it boils down to which one you guys want / prefer to merge on which branch, but yes, if this PR gets in I will cancel the other one.

@dmp42 dmp42 referenced this issue Jan 18, 2014

Closed

Fixing #525 #526

nzakas added a commit that referenced this issue Jan 20, 2014

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Jan 20, 2014

Member

This was completed in #549

Member

nzakas commented Jan 20, 2014

This was completed in #549

@nzakas nzakas closed this Jan 20, 2014

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

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