Skip to content

Conversation

@RichardBradley
Copy link

This PR is a work in progress towards some new scalariform rules to help auto-format overly long lines.

I've started by introducing two new settings: "rightMargin" (default 100) and "reflowComments" (default "true").

So far this will reflow Scaladoc style comments if they are over 100 chars wide.
This works and is tested, so is ready to merge now if you like, or fine to wait for the extra features discussed below.
It includes #57 and #58 as it would conflict if submitted separately.

I'd like to add support for reflowing / autobreaking of:

  • "//" style comments
  • string literals, i.e.
  val example = "a very long string literal, for example an error message or simliar"
  // would become:
  val example = "a very long string literal, " + 
    "for example an error message or simliar"
  • method calls:
  foo.example(example1, example2, example3, example4, example5, example6, example7)
  // would become:
  foo.example(
    example1, 
    example2, 
    example3,
    example4,
    example5,
    example6,
    example7)

Please could you let me know:

  • If you are happy with the proposed new features, and if you'd merge them in if I were to finish up and submit them here
  • If you are happy with the changes so far
  • If you are happy with these new features to be on by default

Thanks,

Rich

@daniel-trinh
Copy link
Owner

  • Is the rightMargin feature intended to completely format anything that is over 100 characters onto multiple lines, or is it going to be scoped to strings, comments, and method calls? How will the method call reformatting work relative to the alignMethodCalls feature, which can shorten a line indirectly by placing arguments on multiple lines? One resolution to this may be to perform the reflowing first, and then have the behavior of alignMethodCalls kick in afterwards.
  • It might be simpler to just always reflow comments instead of having it be an option -- my experience with having extra scalariform options is that it makes maintaining the codebase more difficult when trying to combine and merge the behavior of overlapping features. What kind of scenario would someone want the rest of their code formatted to be less than 100 characters, but have their comments be longer?
  • I am okay with these features being on by default.

@RichardBradley
Copy link
Author

Is the rightMargin feature intended to completely format anything that is over 100 characters onto multiple lines, or is it going to be scoped to strings, comments, and method calls?

I was planning to have a single int-valued "rightMargin" setting and several boolean-valued settings like "reflow strings", "reflow comments", "reflow method calls" etc.

Do you think we need the option to have different margins for strings, comments and method calls? How might that config look?

How will the method call reformatting work relative to the alignMethodCalls feature, which can shorten a line indirectly by placing arguments on multiple lines? One resolution to this may be to perform the reflowing first, and then have the behavior of alignMethodCalls kick in afterwards.

I'm not sure yet. I was expecting to do all the other formatting first, then add extra breaks only when the line was over-long. After adding extra breaks, I'd then re-apply all the normal formatting.

For example, a one-line method call like

foo(a,b,c)

might get reformatted to just add spaces:

foo(a, b, c)

But only if it went over the right margin would the "reflow method calls" format come into play:

foo(
  a,
  b,
  c)

and then that "reflown" method call would need reformatting according to the current preferences like "alignArguments" (EDIT: and the "dangling close parenthesis" setting, see #29).

It might be simpler to just always reflow comments instead of having it be an option -- my experience with having extra scalariform options is that it makes maintaining the codebase more difficult when trying to combine and merge the behavior of overlapping features.

Currently it's well isolated, so it's easy to have an on/off flag. I think this is likely to stay the same, as there will always be a single "entry point" to the reflowing where the current text is compared to the current margin.

I think it's likely to be a setting where individual preferences will differ. Some people are very keen on strict 80-char lines, and some people think that's pedantic nonsense and are happy with 150-char lines. It's also a more invasive reformatting than many of the rules. IMO this is one that does need an option to control it.

What kind of scenario would someone want the rest of their code formatted to be less than 100 characters, but have their comments be longer?

My plan was to have all the "reflow" formatters controlled by a single "rightMargin" setting, so I think this question doesn't arise, unless I've misunderstood you?

I am okay with these features being on by default.

OK, thanks.

You didn't directly address "if you'd merge them in if I were to finish up and submit them here", but it sounds like you would?

@RichardBradley
Copy link
Author

(I've added support for reflowing "//" style comments to the PR. I'm still working on the other TODOs listed above.)

@ShaneDelmore
Copy link

Great feature addition. Can you add a setting for the closing paren placement?

I would love to see the outcome of a split turn out to be
foo(
a,
b,
c
)

@RichardBradley
Copy link
Author

Can you add a setting for the closing paren placement?

I agree that this needs a setting.
I think that this should be covered by #29 and should be independent of any changes here (although we'd need to ensure that they work together).

@RichardBradley
Copy link
Author

@kiritsuku
Copy link
Collaborator

Hey @RichardBradley, scalariform has new maintainers. Are you interested in bringing this PR back to live? You would have to rebase against the master branch of https://github.com/scala-ide/scalariform

@RichardBradley
Copy link
Author

I'm not working on Scala at the moment (boo), but I might come back to this at some point in the future. My work so far is here if anyone wants to pick this up or use parts of it.

I'll reopen at that location if I return to this.

I think this would be a very valuable feature to some users (but not all), but after reading the posts I linked to above, I'm worried that this might be very difficult or impossible to get right in all cases. Reflowing comments (the work so far) is useful and more approachable than reflowing code.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants