fill-paragraph ill designed

fill-paragraph ill designed

[Andreas Röhler]

Hi guys,

not the first time mentioning this, just another essay. As keeping a 
forked Emacs is costly...

There are many issues, uncertain/unsound decisions inside this piece of 
code.

Let's start with fill-paragraph-function.

If fill-paragraph-function is set, it must be assumed a valid function 
and be used.

So it must precede that clause:

(and region transient-mark-mode mark-active

;;;;

Just to start with,

Andreas

Re: fill-paragraph ill designed

[Tassilo Horn]

Andreas Röhler <andreas.roehler@online.de> writes:

> There are many issues, uncertain/unsound decisions inside this piece
> of code.
>
> Let's start with fill-paragraph-function.
>
> If fill-paragraph-function is set, it must be assumed a valid function
> and be used.

That's not what the documentation says:

,----[ C-h f fill-paragraph RET ]
| fill-paragraph is an interactive compiled Lisp function in ‘fill.el’.
| 
| [...]
| 
| The REGION argument is non-nil if called interactively; in that
| case, if Transient Mark mode is enabled and the mark is active,
| call ‘fill-region’ to fill each of the paragraphs in the active
| region, instead of just filling the current paragraph.
`----

So if there is an active region, `fill-paragraph' just calls
`fill-region', and then the `fill-paragraph-function' is irrelevant.

> So it must precede that clause:
>
> (and region transient-mark-mode mark-active

If `fill-paragraph' would call `fill-paragraph-function' also in this
case, then there are two options:

  (1) `fill-paragraph's documentation stays what it is, and then every
      `fill-paragraph-function' ever going to be defined has to handle
      the active region case and call `fill-region'.

  (2) `fill-paragraph' can't specify any special behavior for the case
      where the region is active.  It might just say that it itself
      behaves differently with an active region when f-p-f is nil.

Option (1) is useless, and option (2) is confusing and might lead to
inconsistent behavior, e.g., in some modes `M-q' with an active region
fills the region (because f-p-f is nil or a function which considers the
active region case) and in some other modes it just fills the current
paragraph (because f-p-f is set to some function which doesn't consider
the active region case).

Bye,
Tassilo

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 25.08.2015 um 11:05 schrieb Tassilo Horn:
> Andreas Röhler <andreas.roehler@online.de> writes:
>
>> There are many issues, uncertain/unsound decisions inside this piece
>> of code.
>>
>> Let's start with fill-paragraph-function.
>>
>> If fill-paragraph-function is set, it must be assumed a valid function
>> and be used.
> That's not what the documentation says:
>
> ,----[ C-h f fill-paragraph RET ]
> | fill-paragraph is an interactive compiled Lisp function in ‘fill.el’.
> |
> | [...]
> |
> | The REGION argument is non-nil if called interactively; in that
> | case, if Transient Mark mode is enabled and the mark is active,
> | call ‘fill-region’ to fill each of the paragraphs in the active
> | region, instead of just filling the current paragraph.
> `----
>
> So if there is an active region, `fill-paragraph' just calls
> `fill-region', and then the `fill-paragraph-function' is irrelevant.

Yeah, but that's unhappy. Where the global fill-paragraph should know 
from, what's needed in a special mode, what special modes want to do 
with strings?

Other way around: any fill-paragraph-function is free to check for 
region as done here.

>> So it must precede that clause:
>>
>> (and region transient-mark-mode mark-active
> If `fill-paragraph' would call `fill-paragraph-function' also in this
> case, then there are two options:
>
>    (1) `fill-paragraph's documentation stays what it is, and then every
>        `fill-paragraph-function' ever going to be defined has to handle
>        the active region case and call `fill-region'.

That's completely up to the mode how to proceed then. There too many 
unforseen cases - just keep the way free, keep Emacs expandable.



>    (2) `fill-paragraph' can't specify any special behavior for the case
>        where the region is active.

If it's all up to fill-paragraph-function, global fill-paragraph would 
be out of the way.


>   It might just say that it itself
>        behaves differently with an active region when f-p-f is nil.
>
> Option (1) is useless, and option (2) is confusing and might lead to
> inconsistent behavior, e.g., in some modes `M-q' with an active region
> fills the region (because f-p-f is nil or a function which considers the
> active region case) and in some other modes it just fills the current
> paragraph (because f-p-f is set to some function which doesn't consider
> the active region case).
>
> Bye,
> Tassilo
>

Cheers,

Andreas

Re: fill-paragraph ill designed

[Tassilo Horn]

Andreas Röhler <andreas.roehler@online.de> writes:

>> So if there is an active region, `fill-paragraph' just calls
>> `fill-region', and then the `fill-paragraph-function' is irrelevant.
>
> Yeah, but that's unhappy. Where the global fill-paragraph should know
> from, what's needed in a special mode, what special modes want to do
> with strings?

Indeed, I had also expected that `fill-region' would call
`fill-paragraph-function' on each paragraph in the region which it
doesn't.  But its docstring says that modes might want to use some other
hook instead, e.g., `fill-forward-paragraph-function'.

,----[ C-h v fill-paragraph-function RET ]
| fill-paragraph-function is a variable defined in ‘fill.el’.
| Its value is message-fill-paragraph
| Local in buffer *unsent wide reply to Andreas Röhler*; global value is nil
| 
|   This variable may be risky if used as a file-local variable.
| 
| Documentation:
| Mode-specific function to fill a paragraph, or nil if there is none.
| If the function returns nil, then ‘fill-paragraph’ does its normal work.
| A value of t means explicitly "do nothing special".
| Note: This only affects ‘fill-paragraph’ and not ‘fill-region’
| nor ‘auto-fill-mode’, so it is often better to use some other hook,
| such as ‘fill-forward-paragraph-function’.
`----

That's its docstring:

,----[ C-h v nil RET ]
| fill-forward-paragraph-function is a variable defined in ‘fill.el’.
| Its value is forward-paragraph
| 
|   This variable may be risky if used as a file-local variable.
| 
| Documentation:
| Function to move over paragraphs used by the filling code.
| It is called with a single argument specifying the number of paragraphs to move.
| Just like ‘forward-paragraph’, it should return the number of paragraphs
| left to move.
`----

Well, that actually only speaks of moving over paragraphs, not about
filling while doing so.  But let's try it out anyway since I could also
use that in GNU AUCTeX.

AUCTeX has a custom `fill-paragraph-function' named
`LaTeX-fill-paragraph'.  In contrast to the normal `fill-paragraph', it
ensures that verbatim macros will never be wrapped, e.g., \verb|foo bar|
will always stay on one line which is very important.  A wrapped
verbatim macro is an error.

And indeed, when I mark multiple paragraphs and do M-q, I get normal
filling which also wrapps inside \verb stuff and thus breaks my LaTeX
code. :-(

Ok, so then I tried setting `fill-forward-paragraph-function'
buffer-locally to a new function:

--8<---------------cut here---------------start------------->8---
(defun LaTeX-fill-forward-paragraph (arg)
  (LaTeX-fill-paragraph)
  (forward-paragraph arg))
--8<---------------cut here---------------end--------------->8---

However, that doesn't change anything.  Or concretely, `fill-region'
first formats the paragraphs in the region as I'd like to have it using
`LaTeX-fill-paragraph' on each paragraph, but finally the (and ...) in
the condition

--8<---------------cut here---------------start------------->8---
	(if (and (>= (point) initial) (< (point) end))
	    (setq fill-pfx
		  (fill-region-as-paragraph (point) end justify nosqueeze))
	  (goto-char end))))
--8<---------------cut here---------------end--------------->8---

is true and thus `fill-region-as-paragraph' is called which messes up my
correctly filled region.

I can work around that by moving point out of the current paragraph
after filling:

--8<---------------cut here---------------start------------->8---
(defun LaTeX-fill-forward-paragraph (arg)
  (LaTeX-fill-paragraph)
  (let ((x (forward-paragraph arg)))
    (forward-char (if (> arg 0) 1 -1))
    x))
--8<---------------cut here---------------end--------------->8---

This seems to do the trick but is really not too obvious.

So now I'm curious, too.  What's the right way for a mode to define its
own filling rules?

Bye,
Tassilo

Re: fill-paragraph ill designed

[Stefan Monnier]

I agree that there's some serious problem there.

For me, the main issue is that fill-paragraph-function isn't used (and
can't be used) by fill-region.

So I'm in favor of obsoleting fill-paragraph-function and replacing it
with something better designed (the introduction of
fill-forward-paragraph-function was a step in that direction, but it
doesn't cover all use cases of fill-paragraph-function).


        Stefan

Re: fill-paragraph ill designed

[Stefan Monnier]

> `LaTeX-fill-paragraph'.  In contrast to the normal `fill-paragraph', it
> ensures that verbatim macros will never be wrapped, e.g., \verb|foo bar|
> will always stay on one line which is very important.  A wrapped
> verbatim macro is an error.

Isn't that what fill-nobreak-predicate is for?

> So now I'm curious, too.  What's the right way for a mode to define its
> own filling rules?

For fill-region?  There isn't any that can cover all known cases like
fill-paragraph-function does for fill-paragraph :-(


        Stefan

Re: fill-paragraph ill designed

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

There is a lot of ugliness in the way fill-paragraph works,
but developing a replacement that works ok in practice is harder
than it looks.  I recall a time that we tried.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 26.08.2015 um 00:14 schrieb Richard Stallman:
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> There is a lot of ugliness in the way fill-paragraph works,
> but developing a replacement that works ok in practice is harder
> than it looks.  I recall a time that we tried.
>

What would prevent switching the clauses as suggested?

Re: fill-paragraph ill designed

[Tassilo Horn]

Andreas Röhler <andreas.roehler@online.de> writes:

>> There is a lot of ugliness in the way fill-paragraph works, but
>> developing a replacement that works ok in practice is harder than it
>> looks.  I recall a time that we tried.
>
> What would prevent switching the clauses as suggested?

IMO, it makes completely sense that `fill-paragraph' delegates to
`fill-region' if the region is active so that not every mode-specific
`fill-paragraph-function' has to do that on its own.

What I think is wrong is the way `fill-region' works right now.  Why
doesn't it move over the paragraphs in the active region using
`fill-forward-paragraph' (which can be defined mode-specific using
`fill-forward-paragraph-function'), and then call
`fill-paragraph-function' on each paragraph?

The current behavior is ok when there is no `fill-paragraph-function',
but if there is, then it should be used.

That would allow mode authors to define a custom
`fill-paragraph-function' (and a `fill-forward-paragraph-function' if
needed), and then multi-paragraph filling would just work.

If there are good reasons for the way it is currently implemented, then
the docs should at least document how a mode author should implement
region/multi-paragraph filling.  The function
`LaTeX-fill-forward-paragraph' in my previous mail seems to work, but it
relies on completely undocumented behavior:

  (1) `fill-forward-paragraph-function' only speaks of moving over
      paragraphs, but `LaTeX-fill-forward-paragraph' fills while moving.
  (2) `LaTeX-fill-forward-paragraph' also moves point one char more than
      required in order not to trap into the last `fill-region'
      condition which would call `fill-region-as-paragraph' and then
      destroy the correct filling again.

Bye,
Tassilo

Re: fill-paragraph ill designed

[Tassilo Horn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> `LaTeX-fill-paragraph'.  In contrast to the normal `fill-paragraph', it
>> ensures that verbatim macros will never be wrapped, e.g., \verb|foo bar|
>> will always stay on one line which is very important.  A wrapped
>> verbatim macro is an error.
>
> Isn't that what fill-nobreak-predicate is for?

Oh yes, seems so.  Thanks for the pointer.

Bye,
Tassilo

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 26.08.2015 um 09:08 schrieb Tassilo Horn:
> Andreas Röhler <andreas.roehler@online.de> writes:
>
>>> There is a lot of ugliness in the way fill-paragraph works, but
>>> developing a replacement that works ok in practice is harder than it
>>> looks.  I recall a time that we tried.
>> What would prevent switching the clauses as suggested?
> IMO, it makes completely sense that `fill-paragraph' delegates to
> `fill-region' if the region is active so that not every mode-specific
> `fill-paragraph-function' has to do that on its own.

Let's assume region is up to or inside a documentation-string in Python.

Then fill-region will be called instead of fill-paragraph-function.

But how should fill-region get access or know about Python docu-string 
styles?

Re: fill-paragraph ill designed

[Tassilo Horn]

Andreas Röhler <andreas.roehler@online.de> writes:

>>>> There is a lot of ugliness in the way fill-paragraph works, but
>>>> developing a replacement that works ok in practice is harder than it
>>>> looks.  I recall a time that we tried.
>>> What would prevent switching the clauses as suggested?
>> IMO, it makes completely sense that `fill-paragraph' delegates to
>> `fill-region' if the region is active so that not every mode-specific
>> `fill-paragraph-function' has to do that on its own.
>
> Let's assume region is up to or inside a documentation-string in
> Python.
>
> Then fill-region will be called instead of fill-paragraph-function.
>
> But how should fill-region get access or know about Python docu-string
> styles?

`fill-region' would utilize `fill-forward-paragraph-function' to move
over the paragraphs in that region and call `fill-paragraph-function' on
them.  Then you as a mode author are free to define what a paragraph in
python means and how to properly fill it, e.g., every function
definition is a "paragraph", and when filling a paragraph, your
`fill-paragraph-function' actually only fills the documentation string
of the function.

Bye,
Tassilo

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 26.08.2015 um 14:40 schrieb Tassilo Horn:
> Andreas Röhler <andreas.roehler@online.de> writes:
>
>>>>> There is a lot of ugliness in the way fill-paragraph works, but
>>>>> developing a replacement that works ok in practice is harder than it
>>>>> looks.  I recall a time that we tried.
>>>> What would prevent switching the clauses as suggested?
>>> IMO, it makes completely sense that `fill-paragraph' delegates to
>>> `fill-region' if the region is active so that not every mode-specific
>>> `fill-paragraph-function' has to do that on its own.
>> Let's assume region is up to or inside a documentation-string in
>> Python.
>>
>> Then fill-region will be called instead of fill-paragraph-function.
>>
>> But how should fill-region get access or know about Python docu-string
>> styles?
> `fill-region' would utilize `fill-forward-paragraph-function' to move
> over the paragraphs in that region

okay

>   and call `fill-paragraph-function' on
> them.

Can't see that. May you point me at?

Cheers,

Andreas

Re: fill-paragraph ill designed

[Tassilo Horn]

No, I can't point at it. I spoke in the subjunctive, i.e., IMO it would be 
the right thing to do for fill-region but currently it works differently.


On August 26, 2015 18:46:23 Andreas Röhler <andreas.roehler@online.de> wrote:

>
> Am 26.08.2015 um 14:40 schrieb Tassilo Horn:
>> Andreas Röhler <andreas.roehler@online.de> writes:
>>
>>>>>> There is a lot of ugliness in the way fill-paragraph works, but
>>>>>> developing a replacement that works ok in practice is harder than it
>>>>>> looks.  I recall a time that we tried.
>>>>> What would prevent switching the clauses as suggested?
>>>> IMO, it makes completely sense that `fill-paragraph' delegates to
>>>> `fill-region' if the region is active so that not every mode-specific
>>>> `fill-paragraph-function' has to do that on its own.
>>> Let's assume region is up to or inside a documentation-string in
>>> Python.
>>>
>>> Then fill-region will be called instead of fill-paragraph-function.
>>>
>>> But how should fill-region get access or know about Python docu-string
>>> styles?
>> `fill-region' would utilize `fill-forward-paragraph-function' to move
>> over the paragraphs in that region
>
> okay
>
>>   and call `fill-paragraph-function' on
>> them.
>
> Can't see that. May you point me at?
>
> Cheers,
>
> Andreas
>
>

Re: fill-paragraph ill designed

[Stefan Monnier]

>> and call `fill-paragraph-function' on them.
> Can't see that. May you point me at?

fill-region doesn't (and more importantly can't) delegate to
fill-paragraph-function since fill-paragraph-function doesn't take
begin/end arguments.

Tassilo was presenting a hypothetical model.


        Stefan

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 27.08.2015 um 18:35 schrieb Stefan Monnier:
>>> and call `fill-paragraph-function' on them.
>> Can't see that. May you point me at?
> fill-region doesn't (and more importantly can't) delegate to
> fill-paragraph-function since fill-paragraph-function doesn't take
> begin/end arguments.
>
> Tassilo was presenting a hypothetical model.
>
>
>          Stefan
>
>

The latter was confessed already, the former "since" seems not valid.
Wherefrom the implication?

A fill-paragraph-function might do what it wants - taking or ignoring 
arguments.

Re: fill-paragraph ill designed

[Tassilo Horn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> and call `fill-paragraph-function' on them.
>> Can't see that. May you point me at?
>
> fill-region doesn't (and more importantly can't) delegate to
> fill-paragraph-function since fill-paragraph-function doesn't take
> begin/end arguments.

Are you saying that there would be a problem if the region starts in the
middle of a paragraph and/or ends in the middle of another paragraph?

I didn't test but I think that case could be solved by narrowing to the
region before doing the filling.  But of course, then the first and last
line would probably be filled beyond fill-column if the region
starts/ends in the middle of a line.  So the region could be extended or
shrunken to complete lines before narrowing with the negative effect
that two partial lines are either not filled or filled too much.

So yes, it seems to be not possible to get it completely right.  But
IMHO that solution is still better than simply ignoring
`fill-paragraph-function' and filling in a way which may break the
syntax of the text.

Bye,
Tassilo

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 28.08.2015 um 08:30 schrieb Tassilo Horn:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>>> and call `fill-paragraph-function' on them.
>>> Can't see that. May you point me at?
>> fill-region doesn't (and more importantly can't) delegate to
>> fill-paragraph-function since fill-paragraph-function doesn't take
>> begin/end arguments.
> Are you saying that there would be a problem if the region starts in the
> middle of a paragraph and/or ends in the middle of another paragraph?
>
> I didn't test but I think that case could be solved by narrowing to the
> region before doing the filling.  But of course, then the first and last
> line would probably be filled beyond fill-column if the region
> starts/ends in the middle of a line.  So the region could be extended or
> shrunken to complete lines before narrowing with the negative effect
> that two partial lines are either not filled or filled too much.
>
> So yes, it seems to be not possible to get it completely right.  But
> IMHO that solution is still better than simply ignoring
> `fill-paragraph-function' and filling in a way which may break the
> syntax of the text.
>
> Bye,
> Tassilo
>

IMHO fill-region stuff doesn't belong into at all.

Paragraph and region are independent notions - both with its own ideas 
of beginning and end.
Let's keep them separated.

Cheers,

Andreas

Re: fill-paragraph ill designed

[Tassilo Horn]

Andreas Röhler <andreas.roehler@online.de> writes:

> IMHO fill-region stuff doesn't belong into at all.
>
> Paragraph and region are independent notions - both with its own ideas
> of beginning and end.  Let's keep them separated.

Well, then the solution is easy for you.  In your mode's map, remap
`fill-paragraph' to `your-mode-fill-paragraph' instead of defining it as
`fill-paragraph-function', e.g.,

  (define-key your-mode-map [remap fill-paragraph]
              #'your-mode-fill-paragraph)

Then M-q (or whatever the user has configured to be `fill-paragraph')
will call your mode-specific paragraph filling function.

Bye,
Tassilo

Re: fill-paragraph ill designed

[Stefan Monnier]

> So yes, it seems to be not possible to get it completely right.

And it's super yucky!

> But IMHO that solution is still better than simply ignoring
> `fill-paragraph-function' and filling in a way which may break the
> syntax of the text.

But there's a better solution: add a new *-function variable which can
be used both by fill-paragraph and fill-region.


        Stefan

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 28.08.2015 um 10:26 schrieb Tassilo Horn:
> Andreas Röhler <andreas.roehler@online.de> writes:
>
>> IMHO fill-region stuff doesn't belong into at all.
>>
>> Paragraph and region are independent notions - both with its own ideas
>> of beginning and end.  Let's keep them separated.
> Well, then the solution is easy for you.  In your mode's map, remap
> `fill-paragraph' to `your-mode-fill-paragraph' instead of defining it as
> `fill-paragraph-function', e.g.,
>
>    (define-key your-mode-map [remap fill-paragraph]
>                #'your-mode-fill-paragraph)
>
> Then M-q (or whatever the user has configured to be `fill-paragraph')
> will call your mode-specific paragraph filling function.
>
> Bye,
> Tassilo
>
>

Hmm, it's not about my modes.

Suggest the following design of fill-paragraph

(if fill-paragraph-function
            use it

if not:
   (determine beg/end of paragraph by forward-...)
   (fill-region beg end)

That's all.

Re: fill-paragraph ill designed

[Andreas Röhler]

Am 29.08.2015 um 21:47 schrieb Andreas Röhler:
>
>
> Suggest the following design of fill-paragraph
>
> (if fill-paragraph-function
>            use it
>
> if not:
>   (determine beg/end of paragraph by forward-...)
>   (fill-region beg end)
>
> That's all.
>
>

As a first step, which would not interfere with current way,

what about out-factoring the remaining let-clause --(let ((before 
(point))...-- as

just-fill-paragraph

for example.

That would allow a call from outside and also clean-up it a little bit.

Re: fill-paragraph ill designed

[Stefan Monnier]

> As a first step, which would not interfere with current way,
> what about out-factoring the remaining let-clause --(let ((before
> (point))...-- as
> just-fill-paragraph
> for example.
> That would allow a call from outside and also clean-up it a little bit.

When suggesting changes, please explain what problem you're trying
to fix.


        Stefan