Re: Native line numbers landed on master

[Eli Zaretskii <eliz@gnu.org>]

> From: Alex <agrambot@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 09 Jul 2017 16:56:23 -0600
> 
> I've attached a patch below. I changed the menu bar to toggle the global
> mode since the other toggles in the Show/Hide menu are also toggled
> globally. Does it look alright for inclusion?

I have some comments:

> +(defgroup display-line-numbers nil
> +  "Display line numbers in the buffer."
> +  :group 'display)

This means the defcustoms here will be separate from those defined in
cus-start.el.  Is that intended?

More generally, do we want some of the defcustoms to live here and
some in another file?  And if we want all of them here, does that mean
this file needs to be preloaded, or at least auto-loaded when
display-line-numbers is set by the user?

> +(defcustom display-line-number-inhibit-shrink nil

Instead of "inhibit-shrink" (which is a kind-of double negation), I
would suggest to use "grow-only", which AFAIR was your original
suggestion.

> +(defcustom display-line-number-width-start nil
> +  "If non-nil, set initial line number width based on buffer contents.

"Based on buffer contents" is a euphemism.  I would suggest to tell
explicitly that this will cause the lines counted in the buffer when
it's created.

> +To change the type of line numbers displayed by default,
> +customize `display-line-number-type'. To change the type while
                                       ^^
Two spaces, please.

> +(define-globalized-minor-mode global-display-line-numbers-mode
> +  display-line-numbers-mode
> +  (lambda ()
> +    (unless (or (minibufferp)
> +                ;; taken from linum.el
> +                (and (daemonp) (null (frame-parameter nil 'client))))
> +      (display-line-numbers-mode))))

The daemonp part is only needed when display-line-number-width-start
is non-nil, right?

Thanks.