From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: Patch to highlight current line number [nlinum.el] Date: Fri, 15 Jul 2016 22:15:42 -0400 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1468636290 1410 80.91.229.3 (16 Jul 2016 02:31:30 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 16 Jul 2016 02:31:30 +0000 (UTC) Cc: code.falling@gmail.com, Emacs developers To: Kaushal Modi Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jul 16 04:31:22 2016 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bOFNw-0007N3-Ai for ged-emacs-devel@m.gmane.org; Sat, 16 Jul 2016 04:31:20 +0200 Original-Received: from localhost ([::1]:35660 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bOF96-0007TF-QS for ged-emacs-devel@m.gmane.org; Fri, 15 Jul 2016 22:16:00 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46496) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bOF8x-0007T4-QT for emacs-devel@gnu.org; Fri, 15 Jul 2016 22:15:52 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bOF8r-0007kS-On for emacs-devel@gnu.org; Fri, 15 Jul 2016 22:15:50 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:11246) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bOF8r-0007kK-IN for emacs-devel@gnu.org; Fri, 15 Jul 2016 22:15:45 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0A+FgA731xV/3mcpUVcgxCEAoVVwwsEAgKBPD0QAQEBAQEBAYEKQQWDXQEBAwFWIwULCzQSFBgNJIg3CM8jAQEIAgEfizqFBQeELQWQNI5jjhaGEoFFI4Fmgi4igngBAQE X-IPAS-Result: A0A+FgA731xV/3mcpUVcgxCEAoVVwwsEAgKBPD0QAQEBAQEBAYEKQQWDXQEBAwFWIwULCzQSFBgNJIg3CM8jAQEIAgEfizqFBQeELQWQNI5jjhaGEoFFI4Fmgi4igngBAQE X-IronPort-AV: E=Sophos;i="5.13,465,1427774400"; d="scan'208";a="248146153" Original-Received: from 69-165-156-121.dsl.teksavvy.com (HELO fmsmemgm.homelinux.net) ([69.165.156.121]) by ironport2-out.teksavvy.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 15 Jul 2016 22:15:43 -0400 Original-Received: by fmsmemgm.homelinux.net (Postfix, from userid 20848) id CA29CAE0ED; Fri, 15 Jul 2016 22:15:42 -0400 (EDT) In-Reply-To: (Kaushal Modi's message of "Fri, 15 Jul 2016 17:10:11 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.181 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:205750 Archived-At: > Inspired by the nlinum-relative package[1], I have come up with the below > patch for nlinum that just sets the current line number in a different face > `nlinum-current-line-face'. @SheJinxin, hope this is fine with you. Sounds like a good feature, thanks. > By submitting this patch to the list, I am looking forward to how the > performance can be improved. See my comments below. You might like to look at nhexl-mode to see how I've done that in a similar context (tho the nhexl-mode approach is not really right either). > - :lighter nil ;; (" NLinum" nlinum--desc) > + :lighter nil ; (" NLinum" nlinum--desc) Nitpick: this change results in code that mis-indented because a single ";" should be indented to comment-column or thereabout (use M-; to place it right). I wanted the comment close to the code which is why I used ";;". > + (add-hook 'post-command-hook #'nlinum--current-line-update nil t)) Using post-command-hook is OK (that's what I use in nhexl-mode as well). I think it'd be better to use pre-redisplay-functions, but I haven't played with that option yet, and post-command-hook is easier to work with. In any case, the hard thing to get right (which you haven't tried to solve and neither have I in nhexl-mode) is when the buffer is displayed in several windows (in which case each window will want to highlight potentially different line numbers since each window has a different `point`). > +(defun nlinum--current-line-update () > + "Reflush display on current window" > + (when nlinum-mode > + (nlinum--after-change) Why (nlinum--after-change)? > + (setq nlinum--current-line (string-to-number (format-mode-line "%l"))) I think this should use `nlinum--line-number-at-pos`? > + ;; Do reflush only in the visible portion in the window, not the whole > + ;; buffer, when possible. > + (let* ((start (window-start)) > + (end (window-end)) > + (out-of-range-p (< (point-max) end))) > + (when out-of-range-p > + (setq start (point-min)) > + (setq end (point-max))) > + (with-silent-modifications > + (remove-text-properties start end '(fontified)))))) I think this is pessimistic. There's no need to refresh the whole window's line numbers. The only line numbers that can/should change are the line number of the old cursor position and that of the new cursor position. And if point is still in the same line, there's nothing to do. > + (let* ((line-diff (abs (- line nlinum--current-line))) > + (current-line-p (eq line-diff 0)) "...-p" means "..-predicate", and a boolean variable is not a predicate (a predicate in (E)Lisp is usually a function that returns a boolean). And you can simplify this to (is-current-line (= line nlinum--current-line)) > + (if current-line-p > + (put-text-property 0 width 'face 'nlinum-current-line-face str) > + (put-text-property 0 width 'face 'linum str)) Aka (put-text-property 0 width 'face (if current-line-p 'nlinum-current-line-face 'linum) str)) Also I think it'd just always use the `linum` face, as in (put-text-property 0 width 'face (if current-line-p '(nlinum-current-line-face linum) 'linum) str)) Tho it's clearly a question of taste. Stefan