Re: [RFC PATCH] setting indentation styles via `c-file-style' fails to actually change indentation

[Alan Mackenzie <acm@muc.de>]

Hi, Nix.

On Thu, Jun 02, 2011 at 10:43:06PM +0100, Nix wrote:
> On 2 Jun 2011, Alan Mackenzie stated:

> > On Thu, Jun 02, 2011 at 05:47:16PM +0100, Nix wrote:
> >> On 2 Jun 2011, Alan Mackenzie told this:

> >> and have a patch, which works for me, at least... but it's rather ugly,
> >> so if there is a better way which would still allow setting of styles on
> >> file-by-file and directory-(class)-by-directory-(class) basis, then I'm
> >> glad to try that instead!

OK.  Would you please try this patch and let me know how it works.  It's
a bit less ugly than your one (-:


*** cc-mode.el.~4~	2011-06-04 10:32:15.000000000 +0000
--- cc-mode.el	2011-06-05 12:05:12.000000000 +0000
***************
*** 692,698 ****
  		   (c-count-cfss file-local-variables-alist))
  		  (cfs-in-dir-count (c-count-cfss dir-local-variables-alist)))
  	      (c-set-style stile
! 			   (= cfs-in-file-and-dir-count cfs-in-dir-count)))
  	  (c-set-style stile)))
        (when offsets
  	(mapc
--- 692,699 ----
  		   (c-count-cfss file-local-variables-alist))
  		  (cfs-in-dir-count (c-count-cfss dir-local-variables-alist)))
  	      (c-set-style stile
! 			   (and (= cfs-in-file-and-dir-count cfs-in-dir-count)
! 				'keep-defaults)))
  	  (c-set-style stile)))
        (when offsets
  	(mapc


[ BTW, you don't need to separate the patch out of the email.  Just give
patch the entire mail, and it will cope.]


> This happens with trunk Emacs (24.1-to-be).

OK, thanks.  That's the version the above patch applies to.

****************************************************************************

> Not quite. In a file like this:

> int main (int argc, char *argv[])
> {
>     printf ("Hello, world!\n") ;
> }

> with this style in effect:

> (c-add-style "otbs" '("bsd" (c-offsets-alist . ((statement-cont . 4)
>                                                 (inextern-lang . 0)
>                                                 (label . 0)
>                                                 (arglist-cont-nonempty . 4))))))

> and with directory local variables set (in my case) via

> (dir-locals-set-class-variables 'unix '((c-mode . ((c-file-style . "otbs")
>                                                    (fill-column . 80)
>                                                    (indent-tabs-mode . t)))))

> and the file assigned to the 'unix directory class, not a single one of
> the 'otbs style elements take effect. Neither do the inherited 'bsd
> elements. This differs from your case firstly because it tests style
> inheritance, which is all tangled up with this, and secondly because it
> tests dir local variables, which are where the breakage is visible.

OK, thanks.  That's what I needed to know.

> > [ ... ]
> >> DONT-OVERRIDE is clearly doing what it is specified to do. However,
> >> since 'c-set-style' may be called more than once when initializing a
> >> buffer, .....

> > Critical here is what value of `dont-override' is used in each of these
> > calls.

> Well, it's called once at initialization time with the c-default-style
> with dont-override set to 't' iff, oh, gods, you can read that code as
> well as I can .....

I've had several years practice, and I wrote some of it.  ;-)

> ..... and I'm too smashed on antihistamines to parse it right now.

Sorry to hear that.

> Then it gets called again in `c-before-hack-hook' with dont-override
> set to 't' iff the only c-file-style was in the directory local
> variable. (The rationale for this remains opaque to me, but since the
> underlying bug in my case was in the first part I didn't investigate
> that bit too closely. The style doesn't end up set to the directory-
> local style: it ends up set to the default!)

It was an attempt to prevent .dir-local variables from overriding more
specifically set variables (e.g. set in a hook function).  It didn't work
very well.  :-(

> Plus, each of these calls the underlying style-addition function
> repeatedly for inherited styles.

Yes.  That seems safer than trying to separate out what sort of style
setting belongs where, and how to mix them when that's required.

> Maybe the pollen count will go down soon and I can think about this
> more clearly...

Let's hope so.

In the mean time, please forgive me offering you some free advice on bug
reports:

1. Say what you do to cause the bug, not merely what state a program is
in when the bug happens.

2. Be VERY wary about sending "streams of consciousness"; they can
sometimes be difficult to make sense of.  Reading them the morning after
can be helpful.

3. Don't assume the grizzled senile maintainer can simply follow a patch;
explain it!  Often the bug submitter, having just spent several hours on
it, knows the piece of code better than the maintainer.

4. In Emacs, the standard diff format is contextual (-c), but unified
(-u) is OK too.

So, let me know if the patch works, then we can close this bug.

> -- 
> NULL && (void)

-- 
Alan Mackenzie (Nuremberg, Germany).