[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: regression: filling comments in C++ code (today's CVS)
From: |
Stefan Monnier |
Subject: |
Re: regression: filling comments in C++ code (today's CVS) |
Date: |
Mon, 10 Feb 2003 10:36:55 -0500 |
> > > Well, it's not that Kyle E Jones just was being stupid when he wrote
> > > filladapt. He can't do it any better way since there's no hook to
> > > replace fill-context-prefix which implements adaptive-fill-mode.
> >
> > I don't understand. Why can't filladapt check (and call)
> > fill-paragraph-function before doing anything else ?
>
> I.e. filladapt still overrides fill-paragraph but it checks
> fill-paragraph-function earlier itself. You're right, it could do
> that.
>
> CC Mode would still have to use fill-paragraph-function in a fairly
> convoluted way, though:
>
> 1. fill-paragraph is called.
> 2. c-fill-paragraph is called by fill-paragraph-function. It does its
> comment masking etc. Then it dynamically binds
> fill-paragraph-function to nil and calls fill-paragraph again.
> 3. fill-paragraph now runs a second time and performs its default
> action.
> 4. c-fill-paragraph cleans up the masking, restores
> fill-paragraph-function and returns.
> 5. fill-paragraph returns.
The step 4 together with the second half of step 2 amount to
(let (fill-paragraph-function)
(fill-paragraph ...))
and as a matter of fact, the let binding is already done by fill-paragraph
for you (don't know if filladapt does it as well or not, tho), so there's
really nothing special to do other than "return a non-nil value".
That let-binding is also already done by c-fill-paragraph, by the way.
> That'd be workable, but it's not a very pretty solution.
It's the way it's meant to work. In practice (barring compatibility
issues with filladapt, XEmacs, and Emacs < 20.7, i.e. things I don't
know about) it simply looks like the following:
(set (make-local-variable 'fill-paragraph-function) 'foo-fill-para)
and
(defun foo-fill-para (...)
...
(fill-paragraph ...)
...)
where the inner call is typically wrapped in a `let' that rebinds
a few vars such as adaptive-fill-regexp, paragraph-start, ...
or it might directly call fill-region-as-paragraph or do the
filling itself.
> A potential
> problem is if fill-paragraph has some recursion detection (it often
> has, but in the versions I've checked that wouldn't affect this case).
It indeed does and that detection is "set fill-paragraph-function to nil
while calling it" which is exactly what you suggested in steps 2 and 4.
> A cleaner approach would be to make fill-paragraph a function that
> only calls fill-paragraph-function and move the current functionality
> in it to a function that is installed on fill-paragraph-function by
> default. CC Mode could then install a function on
> fill-paragraph-function that chains on to the previous one. (Other
> packages like filladapt would do the same and some effort would be
> necessary in CC Mode to ensure it is installed last, but it's usually
> not very difficult for a major mode to do that.)
I don't see why the current system can't do the exact same thing:
the only difference is that the value corresponding to the default
is nil instead of being fill-paragraph and that instead of
(funcall next-fun ...)
you have to do
(let ((fill-paragraph-function next-fun))
(fill-paragraph ...))
> Anyway, there are compatibility problems with this suggestion, so it's
> probably easier just to live with it.
Indeed.
> > > It'd be nice if fill.el provided a function that only did the pure
> > > text filling between two positions, preferably also with the option to
> >
> > Isn't that what fill-region-as-paragraph does ?
>
> It has adaptive-fill-mode stuff hardcoded. Although easy to shut down
> by covering the adaptive-fill-mode variable, it'd be cleaner to have
> a basic filling function that doesn't have any such "surprises".
I don't necessarily disagree but I think it's a rather minor issue.
> It's also line oriented if I remember correctly, so it can affect
> parts of the lines that are outside the two positions. A fairly
> peculiar case but still something that CC Mode has to do some trickery
> for so that code close by a comment doesn't get filled too.
I think I've tried to improve this, but I can't remember how far I've
gotten. I definitely consider it as a bug when f-r-a-p modifies the
buffer outside of the specified region.
> And for some reason it skips empty lines at the beginning of the
> region, something that should be up to the caller to do. It might
> contain other similar oddities too.
I'm not sure what you mean by "skips empty lines".
> > That would help in the "normal" case, so the question is not "would it
> > work with filladapt" but "would it hurt with filladapt".
> > AFAIK it can't hurt, but I don't know enough about filladapt (or about
> > cc-mode's filling) to be sure.
>
> No, it probably wouldn't. I'll make that change.
Great, thanks,
Stefan