bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Keith David Bershatsky]

Linum-mode could be made more efficient by including conditions to ensure that it only runs one time, instead of 1 to 3 times.  To see exactly what I am talking about, a message can be placed immediately following the let-bound variables in the function `linum-update-window`

(message "point: %s | limit: %s" (point) limit)

This happens for a few reasons.  The `window-scroll-functions` hook is really only needed when point moves outside the visible window limits.  When point is greater than the first value of `(window-end win t)` reported by the initial run of the `window-scroll-functions` hook, the hook runs a second time -- presumably so that redisplay can do its job correctly.  When the `window-scroll-functions` hook runs the second time under that scenario, it gets it right -- i.e., the second time around the `(window-end win t)` reports the correct result.  So there is no point drawing and removing overlays the first time around when it has the wrong `(window-end win t)`.

In addition, the `post-command-hook` function is only needed when point stays within the visible limit limits.

Here is a minor mode that can be used to test the `window-start` and `window-end` values.  A similar system can be used for `linum-mode` -- i.e., use the `linum-update-window` from the `post-command-hook` when point stays within the visible window limits; and, when point moves outside those visible window limits, then use the `window-scroll-functions` hook -- however, limit `linum-update-window` so that it only runs when `(not (> (point) (window-end win t)))`.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; A minor-mode for testing `window-start` and `window-end` BEFORE visual redisplay.

(defvar old-window-start nil
"This local variable is set within the `post-command-hook`; and,
is also used by the `window-scroll-functions` hook.")
(make-variable-buffer-local 'old-window-start)

(defvar old-window-end nil
"This local variable is set within the `post-command-hook`; and,
is also used by the `window-scroll-functions` hook.")
(make-variable-buffer-local 'old-window-end)

(defvar old-window-end-forced nil
"This local variable is set within the `post-command-hook`; and,
is also used by the `window-scroll-functions` hook.")
(make-variable-buffer-local 'old-window-end-forced)

(defvar new-window-start nil
"This local variable is set within the `window-scroll-functions`.")
(make-variable-buffer-local 'new-window-start)

(defvar new-window-end nil
"This local variable is set within the `window-scroll-functions`.")
(make-variable-buffer-local 'new-window-end)

(defun test-post-command-hook ()
"NOT good for things like: `beginning-of-buffer`; `end-of-buffer`; `yank`; etc.
NOTE:  When using `this-command` in conjunction with the `post-command-hook`,
the `window-scroll-functions` hook would need to use `last-command`."
  (when
      (and
        (not (minibufferp))
        (window-live-p (get-buffer-window (current-buffer))))
    (setq old-window-start (window-start))
    (setq old-window-end (window-end))
    (setq old-window-end-forced (window-end nil t))
    (when
        (and
          (not (< (point) old-window-start))
          (not (> (point) old-window-end))
          (not (> (point) old-window-end-forced)))
      (message (concat
        "P.C.H. -- `point`: %s | "
        "`old-window-start`: %s | "
        "`old-window-end`: %s | "
        "`old-window-end-forced`: %s")
          (point)
          old-window-start
          old-window-end
          old-window-end-forced))))

(defun test-window-scroll-functions (win _start)
"Good for things like: `beginning-of-buffer`; `end-of-buffer`; `yank`; etc.
NOTE:  When using `this-command` in conjunction with the `post-command-hook`,
the `window-scroll-functions` hook would need to use `last-command`."
  (when
      (and
        (not (> (point) (window-end win t)))
        old-window-start
        old-window-end
        old-window-end-forced
        (not (minibufferp))
        (window-live-p (get-buffer-window (current-buffer))))
    (when
        (or
          (< (point) old-window-start)
          (> (point) old-window-end)
          (> (point) old-window-end-forced))
      (setq new-window-start _start)
      (setq new-window-end (window-end win t))
      (message (concat
        "W.S.F. -- `point`: %s | "
        "`new-window-start`: %s | "
        "`new-window-end`: %s")
          (point)
          new-window-start
          new-window-end)
        (setq old-window-start nil)
        (setq old-window-end nil)
        (setq old-window-end-forced nil))))

(define-minor-mode test-mode
"A minor-mode for testing `window-start` and `window-end` BEFORE visual redisplay."
  :init-value nil
  :lighter " 𝓣𝓔𝓢𝓣"
  :keymap nil
  :global nil
  (cond
    (test-mode
      (condition-case error
        (progn
          (setq scroll-conservatively 101)
          (setq scroll-margin 0)
          (add-hook 'post-command-hook 'test-post-command-hook nil t)
          (add-hook 'window-scroll-functions 'test-window-scroll-functions nil t)
          (message "Turned ON `test-mode`."))
        (error
         (test-mode 0)
         (signal (car error) (cdr error)))))
    ((not test-mode)
      (remove-hook 'post-command-hook 'test-post-command-hook t)
      (remove-hook 'window-scroll-functions 'test-window-scroll-functions t)
      (message "Turned OFF `test-mode`.") )))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;


In GNU Emacs 24.4.50.1 (x86_64-apple-darwin10.8.0, NS appkit-1038.36 Version 10.6.8 (Build 10K549))
 of 2014-06-01 on MP.local
Repository revision: 117215 lekktu@gmail.com-20140601162519-4dacx2w0ak528z2r
Windowing system distributor `Apple', version 10.3.1038
Configured using:
 `configure --with-ns'

Configured features:
ACL LIBXML2 ZLIB

Important settings:
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  bc-mode: t
  as-mode: t
  ds-mode: t
  ml-mode: t
  sb-mode: t
  sd-mode: t
  tb-mode: t

Recent input:
s-N s-w k <escape> x e m a c s - b u g - r e p o r 
t <return> <S-s-left> r e p o r t - e m a c s - b u 
g <return>

Recent messages:
Updating addresses...done
Starting new Ispell process /Users/HOME/.0.data/.0.emacs/elpa/bin/aspell with english dictionary...
Turned ON `fs-mode`.
Turned ON `vl-mode`.
Type C-c C-x C-z to exit MIME mode, and type C-c C-x ? to get help.
Saving...done
Mark set
[k]ill or [s]ave draft?
Loading msgdb for +/Users/HOME/.0.data/.0.emacs/.0.mail/msgdb/imap/mail.lawlist.com/lawlist/INBOX.Drafts/spool...done
*beep*

Load-path shadows:
/Users/HOME/.0.data/.0.emacs/.0.flim/md4 hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/md4
/Users/HOME/.0.data/.0.emacs/.0.flim/hex-util hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/hex-util
/Users/HOME/.0.data/.0.emacs/.0.flim/sasl hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/net/sasl
/Users/HOME/.0.data/.0.emacs/.0.flim/sasl-ntlm hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/net/sasl-ntlm
/Users/HOME/.0.data/.0.emacs/.0.flim/sasl-digest hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/net/sasl-digest
/Users/HOME/.0.data/.0.emacs/.0.flim/sasl-cram hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/net/sasl-cram
/Users/HOME/.0.data/.0.emacs/.0.flim/ntlm hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/net/ntlm
/Users/HOME/.0.data/.0.emacs/.0.flim/hmac-md5 hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/net/hmac-md5
/Users/HOME/.0.data/.0.emacs/.0.flim/hmac-def hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/net/hmac-def
/Users/HOME/.0.data/.0.emacs/.0.wl/rfc2368 hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/mail/rfc2368
/Users/HOME/.0.data/.0.emacs/.0.wl/utf7 hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/gnus/utf7
/Users/HOME/.0.data/.0.emacs/.0.simi/smime hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/gnus/smime
/Users/HOME/.0.data/.0.emacs/.0.simi/pgg hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/obsolete/pgg
/Users/HOME/.0.data/.0.emacs/.0.simi/pgg-pgp5 hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/obsolete/pgg-pgp5
/Users/HOME/.0.data/.0.emacs/.0.simi/pgg-pgp hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/obsolete/pgg-pgp
/Users/HOME/.0.data/.0.emacs/.0.simi/pgg-parse hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/obsolete/pgg-parse
/Users/HOME/.0.data/.0.emacs/.0.simi/pgg-gpg hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/obsolete/pgg-gpg
/Users/HOME/.0.data/.0.emacs/.0.simi/pgg-def hides /Users/HOME/.0.data/.0.emacs/Emacs_06_01_2014.app/Contents/Resources/lisp/obsolete/pgg-def

Features:
(shadow emacsbug modb-legacy disp-table wl-mime mime-edit pgg-parse
pccl pccl-20 signature mime-setup mail-mime-setup semi-setup mime-pgp
pgg-def mime-play filename mime-image modb-standard elmo-imap4
bbdb-autoloads lawlist-wl wl-demo wl-draft eword-encode wl-template
sendmail elmo-net elmo-cache elmo-map elmo-dop wl-news wl-address
wl-thread wl-folder wl wl-e21 wl-spam wl-action wl-summary wl-refile
wl-util pp elmo-flag elmo-localdir wl-message elmo-mime mmelmo-buffer
mmelmo-imap mime-view mime-conf calist semi-def mmimap mime-parse
mmbuffer mmgeneric elmo-filter elmo-multi elmo-spam elsp-header
elsp-generic elmo elmo-signal wl-highlight wl-vars wl-version
elmo-msgdb modb modb-generic modb-entity luna mime elmo-util emu
invisible inv-23 poem poem-e20 poem-e20_3 eword-decode std11 elmo-date
elmo-vars elmo-version w3m-load mime-w3m w3m browse-url doc-view
jka-compr image-mode w3m-hist w3m-fb bookmark-w3m w3m-ems w3m-ccl ccl
w3m-favicon w3m-image w3m-proc w3m-util smiley gnus-art mm-uu mml2015
mm-view mml-smime smime savehist lawlist-vr-hr lawlist-whitespace
lawlist-github conf-mode log-edit add-log find-lisp package esh-var
esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg eldoc esh-groups
eshell esh-module esh-mode esh-util dired-x view tramp tramp-compat
tramp-loaddefs trampver server grep epa epg epg-config diff-mode
autorevert filenotify log-view pcvs-util ido time-stamp vc-git vc
vc-dispatcher ediff-merg ediff-wind ediff-diff ediff-mult ediff-help
ediff-init ediff-util ediff rx ert ewoc debug eieio-base
lawlist-calculator ps-print ps-def lpr lawlist-flyspell bbdb timezone
find-func dired-aux lawlist-yasnippet help-mode multiple-cursors
mc-separate-operations rectangular-region-mode mc-mark-more thingatpt
mc-cycle-cursors mc-edit-lines multiple-cursors-core rect saveplace
lawlist-tex-mode pcase compile shell pcomplete comint ansi-color ring
skeleton compare-w lawlist-text-mode lawlist-desktop frameset
lawlist-tabbar lawlist-org lawlist-calendar edmacro kmacro derived
lawlist-toodledo advice url-http url-auth url-gw url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util url-parse auth-source eieio byte-opt bytecomp byte-compile
cconv eieio-core password-cache url-vars mailcap json xml noutline
outline easy-mmode gnus-sum gnus-group gnus-undo gnus-start gnus-cloud
nnimap nnmail mail-source tls utf7 mel path-util mime-def alist
mcharset mcs-20 mcs-e20 pcustom pces pces-e20 pces-20 broken poe pym
static apel-ver product netrc nnoo parse-time gnus-spec gnus-int
gnus-range message cl-macs dired format-spec rfc822 mml easymenu
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win gnus
gnus-ems nnheader gnus-util mail-utils mm-util help-fns mail-prsvr
wid-edit cl gv cl-loaddefs cl-lib time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel ns-win tool-bar dnd fontset
image regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode
register page menu-bar rfn-eshadow timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
cocoa ns multi-tty emacs)

Memory information:
((conses 16 801432 72133)
 (symbols 48 56758 0)
 (miscs 40 89 201)
 (strings 32 117816 17437)
 (string-bytes 1 3936631)
 (vectors 16 41143)
 (vector-slots 8 728265 36624)
 (floats 8 1012 201)
 (intervals 56 3671 65)
 (buffers 960 13))

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Eli Zaretskii]

> Date: Sun, 15 Jun 2014 12:11:52 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> Linum-mode could be made more efficient [...]

Can I persuade you (and everyone else) to please forget linum-mode
ever existed, and never to remember that?  If someone wants line
numbers in Emacs, could they please use Stefan's nlinum-mode instead?

linum-mode is a terrible idea squared.  It abuses post-command-hook.
It moves overlays and changes the window's display margins on the fly,
and by that kills almost every redisplay optimization (so don't try
using it if you are accustomed to many windows and many frames,
especially large ones).  It is evil.  We certainly shouldn't make it
more efficient.  It should die a horrible death.  Soon.

Thank you.

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Keith David Bershatsky]

Thank you for the referral to the library `nlinum-mode`.  I've downloaded it and tried it out, and will keep it in my ever growing bag of goodies.  I'll need to try it out over time with my various minor modes to see how they play together.

I understand that the Emacs team may wish to discontinue `linum-mode` in favor of `nlinum-mode`, so this bug report (#17787) can be closed out at your convenience.

I've gone ahead and made a first draft of the fix to `linum-mode`, which is dependent upon the global variable `scroll-margin` being set to zero (as that is the circumstance whereby the `window-scroll-functions` hook makes a second run).  When the `scroll-margin` is set to something like 2 (instead of zero), the condition `(not (> (point) (window-end win t)))` will prevent the function from working.


Thanks,

Keith

(defun linum-update-current ()
  (when
      (and
        (not (minibufferp))
        (window-live-p (get-buffer-window (current-buffer))))
    (setq old-window-start (window-start))
    (setq old-window-end (window-end))
    (setq old-window-end-forced (window-end nil t))
    (when
        (and
          (not (< (point) old-window-start))
          (not (> (point) old-window-end))
          (not (> (point) old-window-end-forced)))
      (linum-update (current-buffer)))))

(defun linum-after-scroll (win _start)
  (when
      (and
        (not (> (point) (window-end win t))) ;; contingent upon `scroll-margin 0`
        old-window-start
        old-window-end
        old-window-end-forced
        (not (minibufferp))
        (window-live-p (get-buffer-window (current-buffer))))
    (when
        (or
          (< (point) old-window-start)
          (> (point) old-window-end)
          (> (point) old-window-end-forced))
      (let* (
          (new-window-start _start)
          (new-window-end (window-end win t)))
        (linum-update (window-buffer win))
        (setq old-window-start nil)
        (setq old-window-end nil)
        (setq old-window-end-forced nil)))))


---------------------------------------

On Jun 15, 2014, at 12:54 PM, Eli Zaretskii wrote:

>> Date: Sun, 15 Jun 2014 12:11:52 -0700
>> From: Keith David Bershatsky <esq@lawlist.com>
>> 
>> Linum-mode could be made more efficient [...]
> 
> Can I persuade you (and everyone else) to please forget linum-mode
> ever existed, and never to remember that?  If someone wants line
> numbers in Emacs, could they please use Stefan's nlinum-mode instead?

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

On Sun, Jun 15, 2014 at 9:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> Can I persuade you (and everyone else) to please forget linum-mode
> ever existed, and never to remember that?  If someone wants line
> numbers in Emacs, could they please use Stefan's nlinum-mode instead?

They don't offer the same features.

     J

[-- Attachment #2: Type: text/html, Size: 474 bytes --]

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Mon, 16 Jun 2014 01:28:36 +0200
> Cc: Keith David Bershatsky <esq@lawlist.com>, 17787@debbugs.gnu.org
> 
> On Sun, Jun 15, 2014 at 9:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Can I persuade you (and everyone else) to please forget linum-mode
> > ever existed, and never to remember that?  If someone wants line
> > numbers in Emacs, could they please use Stefan's nlinum-mode instead?
> 
> They don't offer the same features.

If the missing features in nlinum matter, they should be added.

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 192 bytes --]

On Mon, Jun 16, 2014 at 4:33 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> If the missing features in nlinum matter,

They do, to me.

> they should be added.

Yep. Not entirely trivial, though.

[-- Attachment #2: Type: text/html, Size: 417 bytes --]

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Stefan Monnier]

retitle 17787 Redundant runs of window-scroll-functions
thanks

> Here is a minor mode that can be used to test the `window-start` and
> `window-end` values.  A similar system can be used for `linum-mode` --
> i.e., use the `linum-update-window` from the `post-command-hook` when
> point stays within the visible window limits; and, when point moves
> outside those visible window limits, then use the
> `window-scroll-functions` hook -- however, limit `linum-update-window`
> so that it only runs when `(not (> (point) (window-end win t)))`.

As Eli explained, we're not very interested in improving linum-mode.
I'd rather focus on bringing nlinum-mode to feature-parity with linum-mode.
admittedly, for some uses of linum-mode, the technique used by
nlinum-mode has some inconvenients.

But your code sounds useful, indeed for uses of window-scroll-functions
which don't affect display very much (e.g. don't change window-start).
This said: even if (> (point) (window-end win t)) it may be that
window-scroll-functions won't be called again, because the current
window-scroll-functions may change something in the current buffer
(e.g. add `invisible' properties, shrink the margin, etc...) which cause
(window-end win t) to move past (point).


        Stefan

bug#17787: 24.4.50; Making `linum-mode` more efficient -- no need to run multiple times.

[Stefan Monnier]

>> If the missing features in nlinum matter,
> They do, to me.

BTW, Juanma, could you add those in a "Todo" list of some sort in
nlinum.el?


        Stefan

bug#17787: Redundant runs of window-scroll-functions

[Lars Ingebrigtsen]

Keith David Bershatsky <esq@lawlist.com> writes:

> Linum-mode could be made more efficient by including conditions to
> ensure that it only runs one time, instead of 1 to 3 times.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

We now have `display-line-numbers-mode' (which is very efficient), so I
don't think we'll be improving the efficiency of linum-mode, and I'm
therefore closing this bug report.

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