Re: paragraph-start and fill-paragraph in texinfo-mode

[Joseph Kiniry]

Hello Stefan, Alan, and Richard,

Stefan Monnier <address@hidden> writes:

>> I define my own programming styles and use c-add-style to add them to
>> c-style-alist.  If I set the &optional set-p in the application of
>> c-add-style, then c-setup-paragraph-style sets paragraph-start as if
>> it were not a buffer local variable.  Thus, all buffers henceforth
>> have comment-line-prefix and c-paragraph-start appended to their 
>> paragraph-start.
>
> Would the patch below do the trick?

It looks like it; see below.

> I always hate it when the make-local-variable is separated from the setq,
> precisely because of the risk of introducing the kind of bug above.

I have the same proclivity.

>         Stefan
>
>
> Index: lisp/progmodes/cc-styles.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-styles.el,v
> retrieving revision 1.35
> diff -u -r1.35 cc-styles.el
> --- lisp/progmodes/cc-styles.el       1 Aug 2005 08:37:49 -0000       1.35
> +++ lisp/progmodes/cc-styles.el       12 Sep 2005 18:15:36 -0000
> @@ -498,17 +498,19 @@
>    (let ((comment-line-prefix
>        (concat "[ \t]*\\(" c-current-comment-prefix "\\)[ \t]*")))
>  
> -    (setq paragraph-start (concat comment-line-prefix
> +    (set (make-local-variable 'paragraph-start)
> +         (concat comment-line-prefix
>                                 c-paragraph-start
>                                 "\\|"
> -                               page-delimiter)
> -       paragraph-separate (concat comment-line-prefix
> +                 page-delimiter))
> +    (set (make-local-variable 'paragraph-separate)
> +         (concat comment-line-prefix
>                                    c-paragraph-separate
>                                    "\\|"
> -                                  page-delimiter)
> -       paragraph-ignore-fill-prefix t
> -       adaptive-fill-mode t
> -       adaptive-fill-regexp
> +                 page-delimiter))
> +    (set (make-local-variable 'paragraph-ignore-fill-prefix) t)
> +    (set (make-local-variable 'adaptive-fill-mode) t)
> +    (set (make-local-variable 'adaptive-fill-regexp)
>         (concat comment-line-prefix
>                 (if (default-value 'adaptive-fill-regexp)
>                     (concat "\\("
> @@ -518,8 +520,7 @@
>  
>      (when (boundp 'adaptive-fill-first-line-regexp)
>        ;; XEmacs (20.x) adaptive fill mode doesn't have this.
> -      (make-local-variable 'adaptive-fill-first-line-regexp)
> -      (setq adaptive-fill-first-line-regexp
> +      (set (make-local-variable 'adaptive-fill-first-line-regexp)
>           (concat "\\`" comment-line-prefix
>                   ;; Maybe we should incorporate the old value here,
>                   ;; but then we have to do all sorts of kludges to
> @@ -626,5 +627,5 @@
>  
>  (cc-provide 'cc-styles)
>  
> -;;; arch-tag: c764f61a-96ba-484a-a68f-101c0e9d5d2c
> +;; arch-tag: c764f61a-96ba-484a-a68f-101c0e9d5d2c
>  ;;; cc-styles.el ends here
> Index: lisp/progmodes/cc-mode.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-mode.el,v
> retrieving revision 1.42
> diff -u -r1.42 cc-mode.el
> --- lisp/progmodes/cc-mode.el 1 Aug 2005 08:37:49 -0000       1.42
> +++ lisp/progmodes/cc-mode.el 12 Sep 2005 18:15:36 -0000
> @@ -395,11 +395,6 @@
>    (make-local-variable 'comment-end)
>    (make-local-variable 'comment-start-skip)
>    (make-local-variable 'comment-multi-line)
> -  (make-local-variable 'paragraph-start)
> -  (make-local-variable 'paragraph-separate)
> -  (make-local-variable 'paragraph-ignore-fill-prefix)
> -  (make-local-variable 'adaptive-fill-mode)
> -  (make-local-variable 'adaptive-fill-regexp)
>  
>    ;; now set their values
>    (setq parse-sexp-ignore-comments t
> @@ -1179,5 +1174,5 @@
>  
>  (cc-provide 'cc-mode)
>  
> -;;; arch-tag: 7825e5c4-fd09-439f-a04d-4c13208ba3d7
> +;; arch-tag: 7825e5c4-fd09-439f-a04d-4c13208ba3d7
>  ;;; cc-mode.el ends here

I have applied your patch to my local install.

Alan Mackenzie <address@hidden> writes:

> Hi, Joseph!
>
> On Mon, 12 Sep 2005, Joseph Kiniry wrote:
>
>>Hi Richard et al,
>
> [ .... ]
>
>>A grep-tree of all paths in my load-path reveals that the only place
>>the suspicious subexpression "//+\\|\\**" is used (literally) is
>>in cc-vars.el wrt the defcustom of c-comment-prefix-regexp.
>
>>After some debugging, the problem seems to be with
>>c-setup-paragraph-style [it's actually c-setup-paragraph-variables] via
>>c-add-style [actually, c-set-style].  
>
> This problem was reported by somebody else in late June, and has
> been fixed in the CC Mode CVS at Sourceforge.  It didn't seem very
> urgent then (it's been in the source since 4004 BC, and there'd only
> been one complaint about it), so we didn't immediately amend the
> source in the Emacs CVS.  Sorry!  It seems urgent now.  Martin,
> let's do another mini-release!

No need to apologise---it was only 10 minutes of my time, and a
rare bug at that.

>>I define my own programming styles and use c-add-style to add them to
>>c-style-alist.  If I set the &optional set-p in the application of
>>c-add-style, then c-setup-paragraph-style sets paragraph-start as if
>>it were not a buffer local variable.  Thus, all buffers henceforth
>>have comment-line-prefix and c-paragraph-start appended to their 
>>paragraph-start.
>
> Yes, this is bad, and has been fixed, as noted above.  Personally, I
> don't think c-add-style should have this set-p option - Creating a
> style and applying a style to a buffer are two entirely different
> things IMAO.  Joseph, as a user, how would you feel if c-add-style
> were to lose this set-p option?

I personally would not care, but as Richard correctly points out
below, removing it would possibly cause compatibility problems.

>>I find the documentation for dont-override in c-set-style to be obtuse.
>
> It could be better, couldn't it?  Actually, the entire doc-string looks
> like it would benefit from a flame-thrower.  ;-)  As for the (CC Mode)
> manual, it doesn't even mention DONT-OVERRIDE.  I'm revising the manual
> at the moment, so the entry for c-set-style will get fixed in due course.
> I'll have a look at that doc string sometime in the next few days, and
> send you (Joseph) a patch.

Great.

>>Regardless of use, the definition of a new c-style should not impact
>>buffers in other styles, yet it does, due to the use of
>>paragraph-start as a global variable in c-setup-paragraph-style.
>
> Agreed 100%.  It's been fixed, see the patch below.  Again, sorry the fix
> didn't reach the Emacs CVS before the bug bugged you. 
>
>>My apologies, but I am not familiar enough with cc-styles to provide a
>>quick patch, nor would I trust my patch with such a small data set
>>with which to test.  Perhaps the CC Mode maintainers have a position
>>on this situation?  I have CCed them on this note.
>
> Which was much appreciated.  Thank you!
>
> Here's that patch from Sourceforge:
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> Modified Files:
>       Tag: Branch_5_30
>       cc-styles.el 
> Log Message:
> c-set-style and c-setup-paragraph-variables: Abort the command if we're
>   not in a CC Mode buffer.
>
>
> Index: cc-styles.el
> ===================================================================
> RCS file: /cvsroot/cc-mode/cc-mode/cc-styles.el,v
> retrieving revision 5.149.2.6
> retrieving revision 5.149.2.7
> diff -C2 -d -r5.149.2.6 -r5.149.2.7
> *** cc-styles.el      13 Jun 2005 17:26:54 -0000      5.149.2.6
> --- cc-styles.el      24 Jun 2005 19:46:47 -0000      5.149.2.7
> ***************
> *** 357,360 ****
> --- 357,362 ----
>                              (cons c-indentation-style 0)
>                              'c-set-style-history))))))
> +   (or c-buffer-is-cc-mode
> +       (error "Buffer %s is not a CC Mode buffer (c-set-style)" 
> (buffer-name)))
>     (or (stringp stylename)
>         (error "Argument to c-set-style was not a string"))
> ***************
> *** 489,493 ****
>   
>     (interactive)
> ! 
>     (setq c-current-comment-prefix
>       (if (listp c-comment-prefix-regexp)
> --- 491,497 ----
>   
>     (interactive)
> !   (or c-buffer-is-cc-mode
> !       (error "Buffer %s is not a CC Mode buffer 
> (c-setup-paragraph-variables)"
> !          (buffer-name)))
>     (setq c-current-comment-prefix
>       (if (listp c-comment-prefix-regexp)
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>
>>Best,
>>Joe
>
> -- 
> Alan Mackenzie (Munich, Germany)

I have applied your patch to my local install.

"Richard M. Stallman" <address@hidden> writes:

>     Yes, this is bad, and has been fixed, as noted above.  Personally, I
>     don't think c-add-style should have this set-p option - Creating a style
>     and applying a style to a buffer are two entirely different things IMAO.
>     Joseph, as a user, how would you feel if c-add-style were to lose this
>     set-p option?
>
> This would be an incompatible UI change which is not necessary to fix
> the bug, so please don't change it.
>
> By the way, we are making faster progress now towards a release, so it
> could happen that the installation of the new CC mode is the critical
> path.  I hope you can do this soon.

I conform that, with the application of these two patches, I witness
correct behavior of c-add-style with DONT-OVERRIDE set to 't with
my local install.

Joe
-- 
Joseph Kiniry
School of Computer Science and Informatics
UCD Dublin
http://secure.ucd.ie/
http://srg.cs.ucd.ie/