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

eol-last: only one newline (independently from multiple-empty-lines) #4235

Closed
adrienverge opened this Issue Oct 23, 2015 · 12 comments

Comments

Projects
None yet
5 participants
@adrienverge
Contributor

adrienverge commented Oct 23, 2015

With eol-last there is no way to enforce one (and only one) ending newline. Using the no-multiple-empty-lines trick as currently described in eol-last documentation does not stand if one wants to allow 2 or more blank lines inside files (not at the end).

@eslintbot

This comment has been minimized.

Show comment
Hide comment
@eslintbot

eslintbot Oct 23, 2015

Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

eslintbot commented Oct 23, 2015

Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Oct 23, 2015

adrienverge added a commit to adrienverge/eslint that referenced this issue Oct 23, 2015

Update: Add `max` option to eol-last (fixes eslint#4235)
Currently there is no way to specify a rule for having one (and only
one) ending newline. Using the no-multiple-empty-lines trick as
currently described in eol-last documentation does not stand if one
wants to allow 2+ blank lines inside files (not at the end).

This patch adds an optional `max` option to set the maximum number of
newlines allowed at end of files. For instance, requiring one and only
one newline can be achieved using:

    "eol-last": [2, {"max": 1}]

@adrienverge adrienverge changed the title from eol-last: only one newline (indenpendently of multiple-empty-lines) to eol-last: only one newline (indenpendently from multiple-empty-lines) Oct 23, 2015

adrienverge added a commit to adrienverge/eslint that referenced this issue Oct 23, 2015

Update: Add `max` option to eol-last (fixes eslint#4235)
Currently there is no way to specify a rule for having one (and only
one) ending newline. Using the no-multiple-empty-lines trick as
currently described in eol-last documentation does not stand if one
wants to allow 2+ blank lines inside files (not at the end).

This patch adds an optional `max` option to set the maximum number of
newlines allowed at end of files. For instance, requiring one and only
one newline can be achieved using:

    "eol-last": [2, {"max": 1}]
@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Oct 23, 2015

Member

We already have no-max-empty-lines, this suggestion would overlap with that rule.

Member

nzakas commented Oct 23, 2015

We already have no-max-empty-lines, this suggestion would overlap with that rule.

@adrienverge

This comment has been minimized.

Show comment
Hide comment
@adrienverge

adrienverge Oct 23, 2015

Contributor

@nzakas I guess you're talking about no-multiple-empty-lines? As said in the comment these two rules are independent. For example if you want one and only one EOL at the end, but keep the ability to have 2 consecutive blank lines in the middle of your files, there is currently no way to do it.

Contributor

adrienverge commented Oct 23, 2015

@nzakas I guess you're talking about no-multiple-empty-lines? As said in the comment these two rules are independent. For example if you want one and only one EOL at the end, but keep the ability to have 2 consecutive blank lines in the middle of your files, there is currently no way to do it.

@adrienverge

This comment has been minimized.

Show comment
Hide comment
@adrienverge

adrienverge Oct 23, 2015

Contributor

@nzakas Let me give an example : to have one and only one newline at the end, currently one would need:

no-multiple-empty-lines: [2, {max: 1}]
eol-last: 2

Then, having two consecutive blank lines would not be possible anymore; e.g. the following would not pass:

class MyClass {
    // ...
}


class AnotherClassThatIWouldLikeToSpaceFromThePreviousOne {
    // ...
}
Contributor

adrienverge commented Oct 23, 2015

@nzakas Let me give an example : to have one and only one newline at the end, currently one would need:

no-multiple-empty-lines: [2, {max: 1}]
eol-last: 2

Then, having two consecutive blank lines would not be possible anymore; e.g. the following would not pass:

class MyClass {
    // ...
}


class AnotherClassThatIWouldLikeToSpaceFromThePreviousOne {
    // ...
}

adrienverge added a commit to adrienverge/eslint that referenced this issue Oct 23, 2015

Update: Add `max` option to eol-last (fixes eslint#4235)
Currently there is no way to specify a rule for having one (and only
one) ending newline. Using the no-multiple-empty-lines trick as
currently described in eol-last documentation does not stand if one
wants to allow 2+ blank lines inside files (not at the end).

This patch adds an optional `max` option to set the maximum number of
newlines allowed at end of files. For instance, requiring one and only
one newline can be achieved using:

    "eol-last": [2, {"max": 1}]
@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Oct 23, 2015

Member

Ah I see what you're saying, but I'm not sure the proposed solution is the best way to go. You're basically asking us to force eol-last to check how many lines it uses, but that overlaps with no-multiple-empty-lines, so we can do that because it's possible to end up with a configuration where the two rules contradict each other.

In order to accept your proposal, we'd need to make no-multiple-empty-lines not apply to lines at the end of a file (this would be a breaking change). I'm not sure that would make sense to most people, however, as I could envision people still expecting no-multiple-empty-lines to catch that.

A nonbreaking change would be to add an option to no-multiple-empty-lines that essentially says, "use this other max for end-of-file empty lines."

Member

nzakas commented Oct 23, 2015

Ah I see what you're saying, but I'm not sure the proposed solution is the best way to go. You're basically asking us to force eol-last to check how many lines it uses, but that overlaps with no-multiple-empty-lines, so we can do that because it's possible to end up with a configuration where the two rules contradict each other.

In order to accept your proposal, we'd need to make no-multiple-empty-lines not apply to lines at the end of a file (this would be a breaking change). I'm not sure that would make sense to most people, however, as I could envision people still expecting no-multiple-empty-lines to catch that.

A nonbreaking change would be to add an option to no-multiple-empty-lines that essentially says, "use this other max for end-of-file empty lines."

@adrienverge

This comment has been minimized.

Show comment
Hide comment
@adrienverge

adrienverge Oct 23, 2015

Contributor

This is interesting! But I don't see any contradiction nor need to modify no-multiple-empty-lines.

I agree the two rules overlap for ending newlines, but to me it's no problem if these ending newlines have to respect both the rule for the whole file (no-multiple-empty-lines) and, because they are ending the file, the rule eol-last.

Other rules do overlap: take space-after-keywords and space-before-block for instance, or no-octal and no-magic-numbers, or even no-shadow and camelcase. In these cases the concerned nodes/tokens must respect both rules.

Furthermore the proposed change (implemented in this PR) is purely optional so it wouldn't break anything. In the special case when a user sets "eol-last": [2, {"max": 999}] and "no-multiple-empty-lines": [2, {"max": 3}], then the stricter rule will prevail for ending lines.

What do you think?

Contributor

adrienverge commented Oct 23, 2015

This is interesting! But I don't see any contradiction nor need to modify no-multiple-empty-lines.

I agree the two rules overlap for ending newlines, but to me it's no problem if these ending newlines have to respect both the rule for the whole file (no-multiple-empty-lines) and, because they are ending the file, the rule eol-last.

Other rules do overlap: take space-after-keywords and space-before-block for instance, or no-octal and no-magic-numbers, or even no-shadow and camelcase. In these cases the concerned nodes/tokens must respect both rules.

Furthermore the proposed change (implemented in this PR) is purely optional so it wouldn't break anything. In the special case when a user sets "eol-last": [2, {"max": 999}] and "no-multiple-empty-lines": [2, {"max": 3}], then the stricter rule will prevail for ending lines.

What do you think?

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Oct 23, 2015

Member

I'm sorry, your solution isn't acceptable for the reasons I already mentioned.

Member

nzakas commented Oct 23, 2015

I'm sorry, your solution isn't acceptable for the reasons I already mentioned.

@adrienverge

This comment has been minimized.

Show comment
Hide comment
@adrienverge

adrienverge Oct 26, 2015

Contributor

I understand.

Would a maxEOF option in no-multiple-empty-lines be acceptable? Example:

"no-multiple-empty-lines": [2, {"max": 3, "maxEOF": 1}]
Contributor

adrienverge commented Oct 26, 2015

I understand.

Would a maxEOF option in no-multiple-empty-lines be acceptable? Example:

"no-multiple-empty-lines": [2, {"max": 3, "maxEOF": 1}]
@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Oct 26, 2015

Member

Yeah, something like that. @eslint/eslint-team thoughts?

Member

nzakas commented Oct 26, 2015

Yeah, something like that. @eslint/eslint-team thoughts?

@ilyavolodin

This comment has been minimized.

Show comment
Hide comment
@ilyavolodin

ilyavolodin Oct 26, 2015

Member

I think adding another option to no-multiple-empty-lines would be fine. It should default to max value if not specified.

Member

ilyavolodin commented Oct 26, 2015

I think adding another option to no-multiple-empty-lines would be fine. It should default to max value if not specified.

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Oct 26, 2015

Member

Agreed.

Member

nzakas commented Oct 26, 2015

Agreed.

@nzakas nzakas added accepted and removed evaluating labels Oct 26, 2015

adrienverge added a commit to adrienverge/eslint that referenced this issue Oct 26, 2015

Update: Add `maxEOF` to no-multiple-empty-lines (fixes eslint#4235)
Currently there is no way to specify a rule for having one (and only
one) ending newline. Using eol-last only checks if there is at least
one newline.

This patch adds an optional `maxEOF` option to no-multiple-empty-lines
rule. When set, blank lines at the end are treated differently from
lines within the file. When `maxEOF` is omitted, the rule's original
behaviour does not change.

For instance, requiring one and only one newline at the end of file can
be achieved using:

    {
        eol-last: 2,
        no-multiple-empty-lines: [ 2, { max: 2, maxEOF: 1 } ]
    }

@adrienverge adrienverge changed the title from eol-last: only one newline (indenpendently from multiple-empty-lines) to eol-last: only one newline (independently from multiple-empty-lines) Oct 26, 2015

@adrienverge

This comment has been minimized.

Show comment
Hide comment
@adrienverge

adrienverge Oct 28, 2015

Contributor

Thanks @nzakas @ilyavolodin.

I wrote an implementation of maxEOF in PR #4266.

Contributor

adrienverge commented Oct 28, 2015

Thanks @nzakas @ilyavolodin.

I wrote an implementation of maxEOF in PR #4266.

@nzakas nzakas closed this in 13fd818 Oct 28, 2015

nzakas added a commit that referenced this issue Oct 28, 2015

Merge pull request #4266 from adrienverge/issue4235
Update: Add `maxEOF` to no-multiple-empty-lines (fixes #4235)

lizardruss added a commit to codeschool/eslint that referenced this issue Oct 29, 2015

Update: Add `maxEOF` to no-multiple-empty-lines (fixes eslint#4235)
Currently there is no way to specify a rule for having one (and only
one) ending newline. Using eol-last only checks if there is at least
one newline.

This patch adds an optional `maxEOF` option to no-multiple-empty-lines
rule. When set, blank lines at the end are treated differently from
lines within the file. When `maxEOF` is omitted, the rule's original
behaviour does not change.

For instance, requiring one and only one newline at the end of file can
be achieved using:

    {
        eol-last: 2,
        no-multiple-empty-lines: [ 2, { max: 2, maxEOF: 1 } ]
    }

@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.