Re: master 760910f: Add a new buffer-local variable `minor-modes'

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Basil L. Contovounesios]

larsi@gnus.org (Lars Ingebrigtsen) writes:

> branch: master
> commit 760910f4917ad8ff5e1cd1bf0bfec443b02f0e44
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Commit: Lars Ingebrigtsen <larsi@gnus.org>
>
>     Add a new buffer-local variable `minor-modes'

Compiling with --enable-checking=structs fails with:

  pdumper.c: In function ‘dump_buffer’:
  pdumper.c:2696:3: error: #error "buffer changed.
     See CHECK_STRUCTS comment in config.h."
   2696 | # error "buffer changed. See CHECK_STRUCTS comment in config.h."
        |   ^~~~~

Does dump_buffer or similar need updating in pdumper.c?
Either way, its HASH_buffer value should be updated.

Thanks,

-- 
Basil

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Compiling with --enable-checking=structs fails with:
>
>   pdumper.c: In function ‘dump_buffer’:
>   pdumper.c:2696:3: error: #error "buffer changed.
>      See CHECK_STRUCTS comment in config.h."
>    2696 | # error "buffer changed. See CHECK_STRUCTS comment in config.h."
>         |   ^~~~~
>
> Does dump_buffer or similar need updating in pdumper.c?

Thanks.  Hm...  I wonder whether I've confused myself here while doing
the merges.  Did I mean to leave that new member out of the merge?  I'll
check...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Compiling with --enable-checking=structs fails with:
>>
>>   pdumper.c: In function ‘dump_buffer’:
>>   pdumper.c:2696:3: error: #error "buffer changed.
>>      See CHECK_STRUCTS comment in config.h."
>>    2696 | # error "buffer changed. See CHECK_STRUCTS comment in config.h."
>>         |   ^~~~~
>>
>> Does dump_buffer or similar need updating in pdumper.c?
>
> Thanks.  Hm...  I wonder whether I've confused myself here while doing
> the merges.  Did I mean to leave that new member out of the merge?  I'll
> check...

Oh, no, that was a different thing I was thinking of.  I've now fixed
this bit -- I think the correct thing is to set minor_modes to Qnil
here, I guess.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

>>     Add a new buffer-local variable `minor-modes'

I was surprised to find that buffer-local variables take precedence over
let-bound variables in some instances?  If doing

(let ((minor-modes 1))
  (with-current-buffer "some-other-buffer"
    minor-modes))

will return the buffer-local variable in that other buffer, and not the
let-bound one?

Then perhaps that variable should be renamed to something longer and
convoluted...  even if it seemed like a perfect pairing with
`major-mode'/`minor-modes'.

Opinions?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Stefan Monnier]

>     Add a new buffer-local variable `minor-modes'
>     
>     * lisp/emacs-lisp/easy-mmode.el (define-minor-mode): Keep
>     `minor-modes' updated.

Hmm... those files that have already been compiled with earlier Emacsen
will not keep this var up-to-date, so it can't be used reliably.
Also, I wonder why we need this to be a variable.

Can't we have an `enabled-minor-modes` function instead, which can simply
loop through `minor-mode-list` and see which ones are enabled?

> +           ;; Keep `minor-modes' up to date.
> +           (setq minor-modes (delq ',modefun minor-modes))
> +           (when ,getter
> +             (push ',modefun minor-modes))

For global minor modes, this will adjust the list for the buffer that
happens to be current when the mode is changed, but it won't affect all
other buffers's lists.


        Stefan

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Lars Ingebrigtsen]

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

> Hmm... those files that have already been compiled with earlier Emacsen
> will not keep this var up-to-date, so it can't be used reliably.
> Also, I wonder why we need this to be a variable.

Yes, the .elc files will have to be recompiled.

> Can't we have an `enabled-minor-modes` function instead, which can simply
> loop through `minor-mode-list` and see which ones are enabled?

But how do we know which ones are enabled?  As far as I know, there's no
way to know (reliably) -- the :variable that shows whether the mode is
on or not isn't preserved anywhere.

Even if we had a function like that, looping over all possible minor
modes and calling predicates for them sounds hopelessly slow when you
have to do this for thousands of symbols...

>> +           ;; Keep `minor-modes' up to date.
>> +           (setq minor-modes (delq ',modefun minor-modes))
>> +           (when ,getter
>> +             (push ',modefun minor-modes))
>
> For global minor modes, this will adjust the list for the buffer that
> happens to be current when the mode is changed, but it won't affect all
> other buffers's lists.

Oh, darn, didn't think about globalized minor modes...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Oh, darn, didn't think about globalized minor modes...

And there's a difference between globalized and global minor modes, I
guess?  Hm...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Stefan Monnier]

> But how do we know which ones are enabled?  As far as I know, there's no
> way to know (reliably) -- the :variable that shows whether the mode is
> on or not isn't preserved anywhere.

I can't look at it just now, but I remember this problem and have the
vague recollection that I've made some progress on it, but if it's not
sufficient yet, we can and should just fix it.

> Even if we had a function like that, looping over all possible minor
> modes and calling predicates for them sounds hopelessly slow when you
> have to do this for thousands of symbols...

You could do it eagerly just once before calling all the predicate
functions for all the commands, so that'd be cheap.


        Stefan

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Lars Ingebrigtsen]

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

>> Even if we had a function like that, looping over all possible minor
>> modes and calling predicates for them sounds hopelessly slow when you
>> have to do this for thousands of symbols...
>
> You could do it eagerly just once before calling all the predicate
> functions for all the commands, so that'd be cheap.

That's true, but it seems like an unnecessary complication.

For instance, the mode line contains `minor-mode-alist', and has to loop
through it to find the lighters to display.  It could instead work off
of the `minor-mode' list, which is order(s) of magnitude shorter for most
people.

(And whatever mechanism we add for global modes.  Just adding a
`global-minor-modes' variable is the obvious solution, but I was hoping
somebody had a non-obvious alternative.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Stefan Monnier]

> For instance, the mode line contains `minor-mode-alist', and has to loop
> through it to find the lighters to display.  It could instead work off
> of the `minor-mode' list, which is order(s) of magnitude shorter for most
> people.

I have the impression that you have a hammer and you're looking for nails.


        Stefan

Re: master 760910f: Add a new buffer-local variable `minor-modes'

[Lars Ingebrigtsen]

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

>> For instance, the mode line contains `minor-mode-alist', and has to loop
>> through it to find the lighters to display.  It could instead work off
>> of the `minor-mode' list, which is order(s) of magnitude shorter for most
>> people.
>
> I have the impression that you have a hammer and you're looking for nails.

There's nails sticking out all over the place!

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no