From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Karl Fogel Newsgroups: gmane.emacs.devel Subject: Re: Patch to bookmark.el Date: Sat, 17 Dec 2011 16:36:41 -0500 Message-ID: <87zkeqhod2.fsf@floss.red-bean.com> References: Reply-To: Karl Fogel NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: dough.gmane.org 1324157822 5309 80.91.229.12 (17 Dec 2011 21:37:02 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sat, 17 Dec 2011 21:37:02 +0000 (UTC) Cc: emacs-devel@gnu.org To: Matthias Meulien Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Dec 17 22:36:55 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Rc1w6-0006IT-SS for ged-emacs-devel@m.gmane.org; Sat, 17 Dec 2011 22:36:55 +0100 Original-Received: from localhost ([::1]:38813 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rc1w6-0008Jy-Af for ged-emacs-devel@m.gmane.org; Sat, 17 Dec 2011 16:36:54 -0500 Original-Received: from eggs.gnu.org ([140.186.70.92]:52297) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rc1w2-0008Jt-Ka for emacs-devel@gnu.org; Sat, 17 Dec 2011 16:36:52 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rc1w0-0002Dc-6m for emacs-devel@gnu.org; Sat, 17 Dec 2011 16:36:50 -0500 Original-Received: from mail-qy0-f169.google.com ([209.85.216.169]:38007) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rc1w0-0002DW-26 for emacs-devel@gnu.org; Sat, 17 Dec 2011 16:36:48 -0500 Original-Received: by qcsd17 with SMTP id d17so2798495qcs.0 for ; Sat, 17 Dec 2011 13:36:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:references:reply-to:date:in-reply-to :message-id:user-agent:mime-version:content-type; bh=9WU0svptgg9kzvGvN8y/1Vhz2lQ85XE03II0pB1LwnA=; b=Rn82nIAm/wyKs/L2KMRykSpWtrQrl0mjsW/IGZduPiPhLye2eFWqgZIBNgj0752gXJ Sl8k2pwPrdVClxOmryYj2q91VCqmtGpp6TA+z2O+58wSJOqBtiQcp9uk4gIR9UrxaVd6 oxglSclAr12HD7Muzmd0tdkAaIDstOR/CXfwE= Original-Received: by 10.224.17.135 with SMTP id s7mr18563948qaa.65.1324157805681; Sat, 17 Dec 2011 13:36:45 -0800 (PST) Original-Received: from floss.red-bean.com (cpe-66-65-49-129.nyc.res.rr.com. [66.65.49.129]) by mx.google.com with ESMTPS id du5sm28699955qab.14.2011.12.17.13.36.42 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 17 Dec 2011 13:36:44 -0800 (PST) In-Reply-To: (Matthias Meulien's message of "Fri, 9 Dec 2011 17:28:50 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.92 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.216.169 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:146790 Archived-At: --=-=-= Content-Type: text/plain Matthias Meulien writes: >I am sending you a patch to `bookmark.el'. > >The idea is to display the bookmark list and the buffer list ( >`list-buffers') in a consistent way: The default in the buffer list is >now to use the immutable header line. > >Do you think it could be integrated to the trunk? Hi, Matthias. Your patch's direction was reversed, I think. I've inverted it, and converted it to unified diff format ("diff -u"), against the latest version of bookmark.el which can be found here: http://bzr.savannah.gnu.org/lh/emacs/trunk/annotate/head:/lisp/bookmark.el Below I'll annotate your patch with some comments, and at the end will attach a revised patch for review by you and by anyone else -- I'm CC'ing the emacs-devel@ group. Assuming it looks good, I'll commit the revised version soon. === modified file 'lisp/bookmark.el' --- lisp/bookmark.el 2011-11-27 04:43:11 +0000 +++ lisp/bookmark.el 2011-12-17 21:01:27 +0000 @@ -127,6 +127,10 @@ :type 'boolean :group 'bookmark) +(defcustom bookmark-bmenu-use-header-line t + "Non-nil means to use an immovable header-line." + :type 'boolean + :group 'bookmark) This is modeled on `Buffer-menu-use-header-line' in buff-menu.el, right? I think the doc string should say something about how this immovable header would be used instead of the plain text header. The doc string for `bookmark-bmenu-header-height' should be updated to mention which type of header that variable refers to. (defconst bookmark-bmenu-header-height 2 "Number of lines used for the *Bookmark List* header.") @@ -1543,7 +1547,8 @@ (set-buffer buf))) (let ((inhibit-read-only t)) (erase-buffer) - (insert "% Bookmark\n- --------\n") + (if (not bookmark-bmenu-use-header-line) + (insert "% Bookmark\n- --------\n")) (add-text-properties (point-min) (point) '(font-lock-face bookmark-menu-heading)) (dolist (full-record (bookmark-maybe-sort-alist)) @@ -1568,8 +1573,10 @@ (insert "\n"))) (set-buffer-modified-p (not (= bookmark-alist-modification-count 0))) (goto-char (point-min)) - (forward-line 2) (bookmark-bmenu-mode) + (if bookmark-bmenu-use-header-line + (bookmark-bmenu-set-header) + (forward-line bookmark-bmenu-header-height)) (if bookmark-bmenu-toggle-filenames (bookmark-bmenu-toggle-filenames t)))) Looks good. @@ -1578,7 +1585,25 @@ ;;;###autoload (defalias 'edit-bookmarks 'bookmark-bmenu-list) - +(defun bookmark-bmenu-set-header () + "Sets the immutable header line." + (let ((header (concat "%% " "Bookmark"))) + (when bookmark-bmenu-toggle-filenames + (setq header (concat header + (make-string (- bookmark-bmenu-file-column + (- (length header) 3)) ?\s) + "File"))) + (let ((pos 0)) + (while (string-match "[ \t\n]+" header pos) + (setq pos (match-end 0)) + (put-text-property (match-beginning 0) pos 'display + (list 'space :align-to (- pos 1)) + header))) + (put-text-property 0 2 'face 'fixed-pitch header) + (setq header (concat (propertize " " 'display '(space :align-to 0)) + header)) + ;; Code stollen to `buff-menu.el' --Matthias + (setq header-line-format header))) I adjusted the comment to say "Code derived from `buff-menu.el'." (define-derived-mode bookmark-bmenu-mode special-mode "Bookmark Menu" "Major mode for editing a list of bookmarks. @@ -1631,7 +1656,8 @@ (setq bookmark-bmenu-toggle-filenames nil)) (t (bookmark-bmenu-show-filenames) - (setq bookmark-bmenu-toggle-filenames t)))) + (setq bookmark-bmenu-toggle-filenames t))) + (bookmark-bmenu-set-header)) (This is in `bookmark-bmenu-toggle-filenames'.) There's no conditional around the call to `bookmark-bmenu-set-header', so what would happen if `bookmark-bmenu-use-header-line' were nil? I can tell you the answer, because I just tested it -- we get *both* headers :-). I've wrapped it in `(when bookmark-bmenu-use-header-line ...)' to solve the problem. (defun bookmark-bmenu-show-filenames (&optional force) @@ -1696,7 +1722,8 @@ "If point is not on a bookmark line, move it to one. If before the first bookmark line, move to the first; if after the last full line, move to the last full line. The return value is undefined." - (cond ((< (count-lines (point-min) (point)) bookmark-bmenu-header-height) + (cond ((and (not bookmark-bmenu-use-header-line) + (< (count-lines (point-min) (point)) bookmark-bmenu-header-height)) (goto-char (point-min)) (forward-line bookmark-bmenu-header-height)) ((and (bolp) (eobp)) Looks good, except for the long line, which reformatted to stay under 80 columns. Revised patch attached, in unidiff format. --=-=-= Content-Type: text/plain Content-Disposition: inline; filename=meulien-patch-kfogel-revisions.txt Content-Description: Matthias Meulien's patch to modernize bookmark menu header display, with some revisions by Karl Fogel. === modified file 'lisp/bookmark.el' --- lisp/bookmark.el 2011-11-27 04:43:11 +0000 +++ lisp/bookmark.el 2011-12-17 21:32:01 +0000 @@ -127,9 +127,15 @@ :type 'boolean :group 'bookmark) +(defcustom bookmark-bmenu-use-header-line t + "Non-nil means to use an immovable header line, as opposed to inline +text at the top of the buffer." + :type 'boolean + :group 'bookmark) -(defconst bookmark-bmenu-header-height 2 - "Number of lines used for the *Bookmark List* header.") +(defconst bookmark-bmenu-inline-header-height 2 + "Number of lines used for the *Bookmark List* header +\(only significant when `bookmark-bmenu-use-header-line' is nil\).") (defconst bookmark-bmenu-marks-width 2 "Number of columns (chars) used for the *Bookmark List* marks column, @@ -1543,7 +1549,8 @@ (set-buffer buf))) (let ((inhibit-read-only t)) (erase-buffer) - (insert "% Bookmark\n- --------\n") + (if (not bookmark-bmenu-use-header-line) + (insert "% Bookmark\n- --------\n")) (add-text-properties (point-min) (point) '(font-lock-face bookmark-menu-heading)) (dolist (full-record (bookmark-maybe-sort-alist)) @@ -1568,8 +1575,10 @@ (insert "\n"))) (set-buffer-modified-p (not (= bookmark-alist-modification-count 0))) (goto-char (point-min)) - (forward-line 2) (bookmark-bmenu-mode) + (if bookmark-bmenu-use-header-line + (bookmark-bmenu-set-header) + (forward-line bookmark-bmenu-inline-header-height)) (if bookmark-bmenu-toggle-filenames (bookmark-bmenu-toggle-filenames t)))) @@ -1578,7 +1587,25 @@ ;;;###autoload (defalias 'edit-bookmarks 'bookmark-bmenu-list) - +(defun bookmark-bmenu-set-header () + "Sets the immutable header line." + (let ((header (concat "%% " "Bookmark"))) + (when bookmark-bmenu-toggle-filenames + (setq header (concat header + (make-string (- bookmark-bmenu-file-column + (- (length header) 3)) ?\s) + "File"))) + (let ((pos 0)) + (while (string-match "[ \t\n]+" header pos) + (setq pos (match-end 0)) + (put-text-property (match-beginning 0) pos 'display + (list 'space :align-to (- pos 1)) + header))) + (put-text-property 0 2 'face 'fixed-pitch header) + (setq header (concat (propertize " " 'display '(space :align-to 0)) + header)) + ;; Code derived from `buff-menu.el'. + (setq header-line-format header))) (define-derived-mode bookmark-bmenu-mode special-mode "Bookmark Menu" "Major mode for editing a list of bookmarks. @@ -1631,7 +1658,9 @@ (setq bookmark-bmenu-toggle-filenames nil)) (t (bookmark-bmenu-show-filenames) - (setq bookmark-bmenu-toggle-filenames t)))) + (setq bookmark-bmenu-toggle-filenames t))) + (when bookmark-bmenu-use-header-line + (bookmark-bmenu-set-header))) (defun bookmark-bmenu-show-filenames (&optional force) @@ -1696,9 +1725,11 @@ "If point is not on a bookmark line, move it to one. If before the first bookmark line, move to the first; if after the last full line, move to the last full line. The return value is undefined." - (cond ((< (count-lines (point-min) (point)) bookmark-bmenu-header-height) + (cond ((and (not bookmark-bmenu-use-header-line) + (< (count-lines (point-min) (point)) + bookmark-bmenu-inline-header-height)) (goto-char (point-min)) - (forward-line bookmark-bmenu-header-height)) + (forward-line bookmark-bmenu-inline-header-height)) ((and (bolp) (eobp)) (beginning-of-line 0)))) --=-=-=--