Support for re-wrapping comments?

[DanTup]

In VS Code/LSP I'd like to add support for re-wrapping comments (eg. you insert/delete words in a comment that mean they're no longer wrapped nicely at 80 characters, and fixing them manually is a bit of a faff).

There are few ways I could see this working:

• Do it automatically as part of formatting
• Have another command that rewraps the entire file
• Have a command that just rewraps a selection

Rolling it into formatting seems attractive to me (although it'd probably have to be off by default) as "Format Selection" could then format code and re-wrap comments in one action.

I'm not sure whether this would make most sense in dart_style or directly in the analysis server, so I thought it was worth raising for discussion here. Would it fit here? If so, I'm interested in contributing it (though may need some pointers). If not, I'm happy to tackle it in the analysis server.

Thanks!

[srawlins]

I think this is a duplicate of #719, which Bob rolled into #655.

That being said, I like it being a separate request, and discussing here :)

The big issue I raised in #719 is that this feature would require a markdown parser and possibly formatter, so that code in code blocks are not wrapped. I think there are other concerns regarding Markdown, but I'm not certain...

[DanTup]

I think this is a duplicate of #719

Oops, not sure how I missed that (I did search, but apparently not very well!).

The big issue I raised in #719 is that this feature would require a markdown parser

Good point. I was thinking of some basic rules (skipping lines surrounded by triple backticks, blank lines, indented lines, lines starting with - or *) but maybe that's asking for trouble.

If implementing it here is non-trivial (or unlikely to be a priority anytime soon), perhaps having something basic in the analysis server (that just naively wraps a selection and it's up to the user to select the text) is worthwhile?

[munificent]

I suspect it will never be a good idea to do this automatically all the time. Doc comments can contain, well, anything a user wants really, so there are all manner of ways that it might have meaningful newlines that the formatter should preserve but doesn't know how.

I'm potentially open to an option you can use to enable this like a --fix, but I think it will be a lot of work to implement it to the level of quality I'd like to see.

[DanTup]

I'm potentially open to an option you can use to enable this like a --fix

Are those fixes exposed via the API (or could they be)? If not, I think that would prevent the server from using them.

I think it will be a lot of work to implement it to the level of quality I'd like to see.

Yeah, that's fair. Maybe I'll have a go at something separately (or in the server code that's familiar) to see if I can come up with some simple rules that produce reasonable results (I can probably test on the SDK/Flutter repos). If not, then probably it'll need to go in the server as something where the user highlights a selection where the rules won't need to be as solid.

(I don't know how quickly I'll get to it, but I'll post back here when I do)

[munificent]

Are those fixes exposed via the API (or could they be)?

Yes, when you construct a DartFormatter(), you can pass in the list of fixes to apply.

Maybe I'll have a go at something separately (or in the server code that's familiar) to see if I can come up with some simple rules that produce reasonable results (I can probably test on the SDK/Flutter repos).

Cool, keep me posted!

[DanTup]

I had a quick play with this, using some simple rules:

• only rewrap dartdocs (doing whole files with double-slash comments could mess with commented out code - although if the goal was to let the user select a range of comment to invoke this, that could probably be relaxed)
• don't do any lines inside a group of triple-backticks (flip a flag when seeing one and consider it a code block until it flips back)
• don't do any lines that have multiple concurrent spaces (eg. may be indented or manually formatted)
• don't do any lines that start with non-alphanumeric character (eg. may be a list item - foo)
• don't do any lines with backticks or square brackets (see below)

I ran it on a bunch of Flutter framework files with lots of dartdocs (not all, because right now I have to invoke it manually for each file through VS Code) and the results don't seem bad (the comments there are already fairly well formatted, although I think the important question is whether it trashes anything it should not touch):

DanTup/flutter@1391df9

The only real issue I've found so far is due to skipping square brackets/backticks - this was my attempt to be lazy and not have to deal with accidentally wrapping [foo bar](http://) between foo and bar, so we sometimes end up with:

/// a big long line of comment a big long line of comment a big long line of comment
/// with
/// [link](...)

This perhaps could be improved by not skipping those lines, but instead improving the wrapping code to understand not to niavely break on spaces, but treat things in backticks or link format as a single entity. It might be easy to come up with cases where this falls down, although if the goal was support it being manually invoked on a range perhaps it'd be good enough.

Any thoughts? Is it worth experimenting a bit more and/or running it across a larger selection of code?

[munificent]

• don't do any lines inside a group of triple-backticks (flip a flag when seeing one and consider it a code block until it flips back)

You'll probably also want to handle old-style Markdown code blocks that just use four spaces of indentation. Unfortunately, distinguishing those from paragraphs inside a list gets you close to writing a real Markdown parser. :-/

Flutter might not be an ideal corpus for this because they likely only use the latest and greatest idioms.

Any thoughts?

It sounds really promising so far. If it were me, I would lean towards implementing it in the server just because I think it will likely be nicer if users can just indicate "reformat this one comment please". dart_style always wants to work on entire files.

I'm not averse to supporting it as a --fix, but that would be a bigger lift since I'd want to make sure it handles all sorts of Markdown gracefully. For example, if it wraps a list item, the wrapped lines should be indented:

/// *   This is a very long list item that needs to be wrapped so that it doesn't overflow because long lines are bad.

Should become:

/// *   This is a very long list item that needs to be wrapped so that it
///     doesn't overflow because long lines are bad.
/// ^^^^-- note

And, since Markdown supports nested lists and code blocks inside lists, etc. you're really getting towards having an actual Markdown parser in there. I'm not sure if that's a dependency I want dart_style to have.

[DanTup]

You'll probably also want to handle old-style Markdown code blocks that just use four spaces of indentation. Unfortunately, distinguishing those from paragraphs inside a list gets you close to writing a real Markdown parser. :-/

I think this one is covered by the rule for not touching anything with multiple concurrent spaces (since the entire code block would be indented by some). It does mean long list items won't be rewrapped, though I don't know if that's a significant limitation (I figure it's probably ok to support rewrapping a little less and have the user fix up a few things than risk messing things up - as long as we don't undo their fixing if they re-do it). If we do end up using the (or a) markdown parser perhaps this could be improved a little more.

Flutter might not be an ideal corpus for this because they likely only use the latest and greatest idioms.

Yeah, that's true - the code did seem pretty clean already when I was testing. It sounds like it may be worth rigging to be able to run across a whole folder and testing on some other projects, to see if it falls over (if you have suggestions of public repos that may be less recent/consistent and worth testing over, let me know).

I would lean towards implementing it in the server just because I think it will likely be nicer if users can just indicate "reformat this one comment please"

Yep, I think I was leaning the same way. It's probably the only good way to support rewrapping normal double-slash comments (without messing up commented out code).

[munificent]

Leaving a note here that supporting this will likely be somewhat more valuable when we ship the new style (#1253) and every Dart users starts migrating to it. That style change can sometimes change the indentation of lines containing comments and being able to split them if needed could be valuable.

It's less valuable than splitting strings (#909) since long comments usually appear before declarations or statements, and the new style rarely affects the indentation there. But it can come into play if a user has a long comment inside an expression or inside a function expression body inside some larger expression.

As with strings, this is not a reversible change, so if we support it at all, it will have to be with an opt-in flag like --fix.