bug#67718: 30.0.50; prog-fill-reindent-defun vs. fill-paragraph for strings and comments

[Jens Schmidt]

Dmitry Gutov <dmitry@gutov.dev> writes:

>>      |
>>      "on an empty line before a string"
>
> It's not obvious to me that filling would be preferable to
> reindenting, in this case.

This is to retain UX compatibility with the Emacs 29 behavior, where
`fill-paragraph' would fill the current paragraph or the next one, when
on whitespace-only lines before a paragraph.

To what extent does UX compatibility concern you, BTW?

>>        This one could differ in a treesitter mode, I guess.  sql-mode
>>        doesn't even have <-comments, so you always have to move
>>        point*into* a comment to get it filled.  BTW, exactly like
>>        javascript-mode, which is a bit more important, probably:
>
> So we could look for 1-2 more different comment syntaxes, right?
> And maybe string openers.
>
> Would you like to propose a small patch?

No, I only start working for big patches :-)

But more seriously, that might not be fixable by a "small patch", since
AFAICT there is no simple regexp to match an arbitrary two-character
comment starter.  To some extent that is also reflected by the history
of that function, where various generic approaches to detect comments
before point have been tried, but none really successfully, IMO.

So in other words: I can start working on this, but then I'd like to
have a commitment that my work is not spent in vain, even if it comes
out more complex than you'd currently expect.  See below.

>>      (foo bar baz ?\|; baz bar foo)
>>        So this goes the other direction: If you M-q here and
>> expect the
>>        line or surrounding function to be indented, you will get
>>        disappointed, because the heuristics detects this as a comment
>>        that needs to be filled.
>
> In this case it's easy enough to understand what's going on, I
> think, and move out of the string. But other suggestions welcome.

I tried to come up with something easy that'd avoid the edge cases from
my previous post, but that turned out to be harder than I thought.
So the rules are more complex than what we have right now in Emacs 30,
but I hope they fulfill at least the principle of least surprise.

So I'd fill in function `prog-fill-reindent-defun'

  - unless the user has opted out from that (still to be discussed in
    bug#67462) or

  - if point is on or before (UX compatibility!) a line consisting
    solely of one, possibly partial contiguous string or comment or

  - if point is properly inside a string or a /* ... */-style comment
    already spanning more than one line or

  - if point is properly inside a //-style or ;-style comment.

Otherwise I'd reindent.  Examples:

  |
  "fill this
  string"

  "fill |this
  string"

  "fill this
  string"|

  "fill |this string"

  "fill this string"|

  |
  // fill this
  // comment

  // fill |this
  // comment

  // fill this
  // comment|

  // fill |this comment

  // fill this comment|

  |
  /* fill this
  comment */

  /* fill |this
  comment */

  /* fill this
  comment */|

  /* fill |this comment */

  /* fill this comment */|

  /* do |not fill */ /* non-contiguous comment */

  foo = "fill this|
  string";

  foo = /* fill this|
  comment */ 123;

  foo = "do not |fill";

  foo = /* do not |fill */ 123;

  (foo bar baz) |; do not fill

  (foo bar baz) ;| fill this comment

  select 1 from foo |-- do not fill

  select 1 from foo -|- do not fill

  select 1 from foo --| fill this comment

What do you think?

It should be possible to implement that with reasonable effort, but the
result would definitely look less appealing than what we currently have.

Plus we haven't discussed the treesitter part yet ...