bug#21072: Brave new mark-defun (and a testing tool)

[Noam Postavsky]

On Fri, Apr 7, 2017 at 4:25 AM, Marcin Borkowski <mbork@mbork.pl> wrote:

>>> +** New behavior of 'mark-defun' implemented
[...]
>>> +line.
>>> +
>>> ++++
>>> * Changes in Specialized Modes and Packages in Emacs 26.1
>>>
>>
>> This entry should go before the page separator, and the "+++" should go
>> on the line just above the entry, not after it.
>
> That one I do not understand.  This means that "+++" goes essentially
> _to the previous entry_, which doesn't seem to make sense (especially
> when viewing NEWS folded, which I assume everyone does, right?).

Hmm, no, I wasn't really aware of folding (you mean via
`outline-hide-sublevels', right?).

I guess the convention wasn't made with folding in mind, but the "+++"
and "---" markup is just temporary anyway.

>
>>> +(defun beginning-of-defun-comments (&optional arg)
>>
>>> +  (let (nbobp)
>>> +    (while (progn
>>> +             (setq nbobp (zerop (forward-line -1)))
>>> +             (and (not (looking-at "^\\s-*$"))
>>> +                  (beginning-of-defun--in-emptyish-line-p)
>>> +                  nbobp)))
>>> +    (when nbobp
>>> +      (forward-line 1))))
>>
>>
>> The looking-at call is redundant, right?  Anyway, can't that all be
>> replaced by just
>>
>>     (forward-comment (- (point)))
>>     (unless (bolp)
>>       (forward-line 1))
>
> My tests say no.

Oh, right, I thought it was doing backward-comment, but the difference
is that it stops at blank lines, thus the *non-redundant* looking-at
call.

I wonder if that's a sensible thing to do for languages that have
multiline comments though, e.g. Javascript:

/*

This function returns 0

*/
function foo () {
    return 0;
}

Although we might say that such comments should have "*" on the empty lines.

>
>>> +(defun mark-defun (&optional arg)
>>
>>> +  (let (nbobp)
>>> +    (while (progn
>>> +             (setq nbobp (zerop (forward-line -1)))
>>> +             (and (looking-at "^\\s-*$")
>>> +                  nbobp)))
>>> +    (when nbobp
>>> +      (forward-line 1))))
>>
>> I think this can be just
>>
>>     (skip-chars-backward "[:space:]\n")
>>     (unless (bolp)
>>       (forward-line 1))
>
> This OTOH does pass my tests, though I guess it would be clearer to
> replace (bolp) with (bobp) in the above code (if I understand correctly,
> in this situation they should be equivalent).  WDYT?

Yes, I believe they are equivalent. I guess using bobp would explain
better when this happens, though I feel bolp better explains why we're
doing forward-line. I don't think it matters very much either way, go
with whichever you like best.