Subject: Re: bug#2034: [PATCH] 27.0.50; Support mode line constructs for `mode-name' in c-mode

From: Phil Sainty <[email-masked]>
Date: Sun, 8 Jul 2018 14:46:49 +1200

I've updated branch origin/fix/bug-2034 based on recent discussions.

> * lisp/progmodes/cc-vars.el (c-modeline-display-flags): New user
> option.
> * lisp/progmodes/cc-cmds.el (c-modeline-flags): New variable.
> (c--modeline-major-mode): New internal buffer-local variable.
> (c-update-modeline): Use mode line constructs, rather than string
> concatenation, to optionally include minor mode flags in 'mode-name'.

As per Stefan's suggestion, `c-modeline-flags' is now a global
variable containing a mode line construct which will render
appropriately in any buffer.  Its value is:

(c-modeline-display-flags
 ("/"
  (c-block-comment-flag "*" "/")
  (c-electric-flag
   ("l"
    (c-auto-newline "a")))
  (c-hungry-delete-key "h")
  (c-subword-mode "w")))

As a consequence of the change above, we no longer need to call
`c-update-modeline' when toggling the state of one of those minor
modes in a buffer, hence:

> * lisp/progmodes/cc-cmds.el (c-toggle-auto-newline)
> (c-toggle-hungry-state, c-toggle-auto-hungry-state)
> (c-toggle-electric-state, c-toggle-comment-style):
> * lisp/progmodes/cc-mode.el (c-electric-indent-mode-hook)
> (c-electric-indent-local-mode-hook): Remove redundant calls to
> 'c-update-modeline'.  It is no longer necessary to call this function
> every time one of the minor mode states changes.  The remaining calls
> are in 'c-basic-common-init' (which is called via 'c-common-init' by
> all the major modes defined in cc-mode.el), and in the :after-hook of
> those modes (which ensures that 'mode-name' is still processed for a
> derived mode that doesn't call 'c-common-init' itself).

On 05/07/18 08:11, Alan Mackenzie wrote:
> For fitting in better with CC Mode, please:
> (i) Put c-modeline-display-flags (and any other configuration variables)
> in cc-vars.el rather than cc-cmds.el.
> (ii) In cc-mode.info, make a second @vindex entry for your new variable,
> like all the other variables have two @vindexes.
>
> Also, in chapter "Minor Modes", I'd be happier if the paragraph
> beginning "@ccmode{} displays the current state" was amended to
> something like "@ccmode{}, by default, displays the current state".

All done.

Do you still have concerns about these changes, Alan?  I've revised
some of the code that you were unhappy with, so please have another
look at the branch.

> As a final point, how is the backward compatibility of this change?
> How many former Emacsen will it work in?

Let me know if there's anything in particular I should be testing.

On 04/07/18 14:41, Eli Zaretskii wrote:
> Give each of the substrings you concatenate a different face, and
> see what happens after concatenation when the result is shown on the
> mode line.

This works; although, as per the `mode-line-format' documentation, if
a variable is used to store the construct then that variable needs to
have a non-nil `risky-local-variable' property, otherwise the text
properties are ignored.

So both of the following work, provided we have:
(put 'c-modeline-flags 'risky-local-variable t)

(defvar c-modeline-flags
  `(c-modeline-display-flags
    (,(propertize "/")
     (c-block-comment-flag
      ,(propertize "*" 'help-echo "Comment style" 'face 'warning)
      ,(propertize "/" 'help-echo "Comment style" 'face 'hi-yellow))
     (c-electric-flag
      (,(propertize "l" 'help-echo "Electric mode" 'face 'hi-green)
       (c-auto-newline
        ,(propertize "a" 'help-echo "Auto-newline mode" 'face 'hi-pink))))
     (c-hungry-delete-key
      ,(propertize "h" 'help-echo "Hungry-delete mode" 'face 'hi-red-b))
     (c-subword-mode
      ,(propertize "w" 'help-echo "Subword mode" 'face 'error)))))

(defvar c-modeline-flags
  '(c-modeline-display-flags
    ((:propertize "/")
     (c-block-comment-flag
      (:propertize "*" help-echo "Comment style" face warning)
      (:propertize "/" help-echo "Comment style" face hi-yellow))
     (c-electric-flag
      ((:propertize "l" help-echo "Electric mode" face hi-green)
       (c-auto-newline
	(:propertize "a" help-echo "Auto-newline mode" face hi-pink))))
     (c-hungry-delete-key
      (:propertize "h" help-echo "Hungry-delete mode" face hi-red-b))
     (c-subword-mode
      (:propertize "w" help-echo "Subword mode" face error)))))

As there's nothing dynamic going on in these particular text
properties, I'd be inclined to use the former approach over the
latter, if properties were to be added.

Obviously the faces used are only for testing.  I'm not sure that
these actually need a face, but the `help-echo' text could be pretty
handy for clarifying what these potentially-mysterious characters in
the mode line actually mean, for users who don't know.

I was also pondering making a mouse click visit "(ccmode) Minor Modes",
as that provides the clearest documentation.  I've referenced this
node a couple of docstrings, but having direct access to it from the
flags in question in the mode line could be even more helpful.

On 04/07/18 11:57, Phil Sainty wrote:
> Grepping for uses of c-update-modeline I found this related comment in
> lisp/progmodes/antlr-mode.el:
>
> (define-derived-mode antlr-mode prog-mode
>   ;; FIXME: Since it uses cc-mode, it bumps into c-update-modeline's
>   ;; limitation to mode-name being a string.
>   ;; '("Antlr." (:eval (cadr (assq antlr-language antlr-language-alist))))
>   "Antlr"

I've added a second commit to the branch which restores this intended
construct for `antlr-mode', which makes that more dynamic than the
`mode-name' that it was using.

-Phil