Re: Native line numbers landed on master

[Eli Zaretskii]

> From: Alex <address@hidden>
> Cc: address@hidden
> Date: Mon, 10 Jul 2017 14:31:46 -0600
> 
> >> +(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?
> 
> I'm not sure if there's a convention for this (built-in feature with a
> Lisp-level mode wrapper) situation, but it might be nice to separate the
> variables that are directly linked to display-line-numbers and ones that
> only are used in the minor mode wrapper.
> 
> What about defining the defgroup in cus-edit.el, making both these
> variables and the ones in cus-start belong to it?

Sounds OK to me, thanks.  I think we do want all the variables to
appear in the same customization group, even if they are defined
separately.

Maybe also add a comment in display-line-numbers.el that some
additional defcustoms are defined in cus-start.el.

> I don't know if the file should be pre/auto-loaded. Is there a reason to
> do so considering linum.el isn't?

linum.el _is_ autoloaded if you invoke linum-mode, no?

The scenario I had in mind was a user doing this:

  M-x set-variable RET display-line-numbers RET t RET

This is a legitimate way of activating the feature in a buffer.  Do we
want then the user to automatically have access to all the
customizations and features in display-line-numbers.el?

> >> +(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?
> 
> I suppose so, but would one want line numbers in that specific buffer
> either way?

I don't know.  Similarity to interactive invocation, maybe?

> I added that part because you added it to linum.el in bd3c6eec.

That was to fix a bug that I think shouldn't happen with the native
implementation, because it doesn't count lines.

> Does the problem affect display-line-numbers?

I don't think so, but it should be easy to test.  I'll take a look.

> Also, I was wondering if setting display-line-number-width in
> pre-command-hook unconditionally is a good idea. I timed it and the
> function itself seemed slightly faster than a let/when approach, but
> describe-variable states that setting it calls set-buffer-redisplay,
> which disables redisplay optimizations. So if I understand this
> correctly, adding the current display-line-numbers-update-width to
> pre-command-hook would disable redisplay optimizations for every
> command.

Yes, it shouldn't be unconditional.

> P.S. I also noticed that the docstring for display-line-numbers doesn't
> describe the 'relative value, or state that 'visual also uses relative
> line numbers.

Thanks, I fixed this.