bug#57376: 28.1; rcirc-fill-flag ignored after 27->28 upgrade

[Philip Kaludercic]

"Trent W. Buck" <trentbuck@gmail.com> writes:

>
> Philip Kaludercic wrote:
>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>> 
>> >
>> > "Trent W. Buck" <trentbuck@gmail.com> writes:
>> >
>> >> I just upgraded from 1:27.1+1-3.1 to 1:28.1+1-2~bpo11+1.2.
>> >> Now, even though (setq rcirc-fill-flag nil) in my .emacs,
>> >> I am getting hard wrapping.
>> >>
>> >> None of these (evaluated in-buffer with M-:) stopped new messages
>> >> being
>> >> hard-wrapped at 70 columns:
>> >>
>> >>    (setq rcirc-fill-flag nil)
>> >>    (auto-fill-mode -1)
>> >>    (defun rcirc-markup-fill (x y)) ; nop out this unwanted
>> >> function
>> >>    (defun rcirc-fill-paragraph (&optional x)) ; nop out this
>> >> unwanted function
>> >
>> > I haven't tested myself, but it seems like the code in this area
>> > was
>> > changed by:
>> >
>> > commit 849e71fd83fa8796198035464897bf2f28f6226c
>> > Author:     Philip Kaludercic <philipk@posteo.net>
>> > AuthorDate: Wed Jun 9 17:55:55 2021 +0200
>> >
>> >     Implement server-time extension
>> >     
>> >     * rcirc.el (rcirc-implemented-capabilities): Add new capability
>> >     (rcirc-print): Insert messages in the right position
>> >     (rcirc-log): Use right time value
>> >     (rcirc-markup-timestamp): Use right time value
>> >
>> > In particular, this:
>> >
>> > -      ;; squeeze spaces out of text before rcirc-text
>> > -      (fill-region fill-start
>> > - (1- (or (next-single-property-change fill-start
>> > - 'rcirc-text)
>> > -                           rcirc-prompt-end-marker)))
>> >
>> > was changed to this:
>> >
>> > +              ;; squeeze spaces out of text before rcirc-text
>> > +              (fill-region (point-min) (point-max))
>> >
>> > But I don't really know the code well.  Adding Philip to the CCs.
>> 
>> The code was reduced to (fill-region (point-min) (point-max)) because
>> the updated insertion algorithm narrows the buffer to the message,
>> that
>> doesn't have to be right before the prompt, as the entire point of
>> the
>> patch is that messages can be inserted anywhere, depending on
>> server-time tag.
>> 
>> As the comment indicates, the intention is to "squeeze
>> [white]spaces",
>> so a possible fix might be to use `canonically-space-region' instead
>> of
>> `fill-region'?
>
> FWIW, I don't want rcirc to change my whitespace either!

Come to think about it, I don't understand the idea bind it either.  I
looked through the git history, and it appears the change was added in
this commit:

        commit 2fbed782a0705d8b6e776926bb4eaa6b8801cfcb
        Author: Eli Zaretskii <eliz@gnu.org>
        Date:   Fri Feb 17 11:19:00 2006 +0000

            (rcirc-connect): Make all arguments optional, and default to global 
variable
            values for unsupplied args.
            (rcirc-get-buffer-create): Fix bug with setting the target.
            (rcirc-any-buffer): Rename from rcirc-get-any-buffer, and include 
test for
            rcirc-always-use-server-buffer-flag here.
            (rcirc-response-formats): Add %N, which is a facified nick.  %n 
uses the
            default face.  Change the ACTION format string.  If the "nick" is 
the server,
            don't print anything for that field.  Comment fixes.
            (rcirc-target-buffer): Don't test 
rcirc-always-use-server-buffer-flag here.
 ~~~>       (rcirc-print): Squeeze extra spaces out of the text before message.
            (rcirc-put-nick-channel): Strip potential "@" char from nick before 
adding
            them to nick table.
            (rcirc-url-regexp): Improve to match address like "foo.com".


It appears to me that since `rcirc-fill-flag' will usually invoke
`fill-region' just a few lines later, it should be possible to just get
rid of the space-squeezing all together?

But it might also be that I made a mistake in the commit that Lars
mentioned previously.  It appears that in the removed code

--8<---------------cut here---------------start------------->8---
-           ;; squeeze spaces out of text before rcirc-text
-           (fill-region fill-start
-                        (1- (or (next-single-property-change fill-start
-                                                             'rcirc-text)
-                                rcirc-prompt-end-marker)))
--8<---------------cut here---------------end--------------->8---

the variable `fill-start' was initially bound to

--8<---------------cut here---------------start------------->8---
        (let ((moving (= (point) rcirc-prompt-end-marker))
-             (old-point (point-marker))
-             (fill-start (marker-position rcirc-prompt-start-marker)))
+             (old-point (point-marker)))
--8<---------------cut here---------------end--------------->8---

But I am not sure I understand what is going on here, since this "fills
the region" between the beginning of the prompt and either the end of
the prompt or the next (nonexistant?) message?  So shouldn't this
have -- prior to my change -- have done nothing, all the time?