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

Tall style: Preserve blank line between line comments and subsequent code? #1415

Closed
munificent opened this issue Feb 29, 2024 · 5 comments · Fixed by #1481
Closed

Tall style: Preserve blank line between line comments and subsequent code? #1415

munificent opened this issue Feb 29, 2024 · 5 comments · Fixed by #1481
Labels
tall Issues related to the new "tall" style (#1253)

Comments

@munificent
Copy link
Member

The new in-progress style removes all blank lines between a comment and subsequent code when it occurs in a block or at the top level. For example, given:

// Comment.

main() {}

It outputs:

// Comment.
main() {}

This is different from the current style where it will preserve a blank line after a comment and the thing that follows it. This is particularly common to separate the initial copyright comment from subsequent code:

// Copyright (c) 2022, the Dart project authors.  Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:analyzer/dart/ast/ast.dart';

Should the new style do that? I suspect the answer is yes.

cc @natebosch @kallentu

@munificent munificent added the tall Issues related to the new "tall" style (#1253) label Feb 29, 2024
@jakemac53
Copy link
Contributor

jakemac53 commented Feb 29, 2024

Are there any situations other than the top level header where we do this? If not, should we just specialize the behavior up until the first non-comment (or is that feasible)?

@kallentu
Copy link
Member

Are there any situations other than the top level header where we do this? If not, should we just specialize the behavior up until the first non-comment (or is that feasible)?

+1

Removing blank lines between comments and top level code is probably fine? I anecdotally never see code that has really used that feature, besides the exception of the copyright header and the next code line. (But it would be good to get that working.)

@srawlins
Copy link
Member

I see often in large files separations like "// something or other methods" and "// fields for such and such" and "// constructors."

If a comment was previously separated from the thing after it, by a blank line, and suddenly becomes quite attached to the very next element, I can see confusion ensuing.

@srawlins
Copy link
Member

Here is a good code search query to look at top-level examples in google3: lang:dart \n//\s.*\n\n pcre:yes; that catches the last line of a lot of copyright notices. lang:dart \n\n//\s.*\n\n pcre:yes catches single-line comments. You can tweak it for two-line comments, or comments indented two spaces.

@kallentu
Copy link
Member

kallentu commented Feb 29, 2024

Ah, I stand corrected. People do use standalone comments for categorizing.

Preserving blank lines between comments & top level code 👍

munificent added a commit that referenced this issue May 13, 2024
Preserving blank lines between comments and code was mostly working
correctly except for one edge case: A blank line between the *first*
comment at the top level of a file and the subsequent code would get
discarded. This fixes that.

Fix #1415.
munificent added a commit that referenced this issue May 13, 2024
Preserving blank lines between comments and code was mostly working
correctly except for one edge case: A blank line between the *first*
comment at the top level of a file and the subsequent code would get
discarded. This fixes that.

Fix #1415.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tall Issues related to the new "tall" style (#1253)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants