From: Karl Fogel <kfogel@red-bean.com>
To: Matthias Meulien <orontee@gmail.com>
Cc: emacs-devel@gnu.org
Subject: Re: Patch to bookmark.el
Date: Sat, 17 Dec 2011 16:36:41 -0500 [thread overview]
Message-ID: <87zkeqhod2.fsf@floss.red-bean.com> (raw)
In-Reply-To: <CAFEQCfBwuU54R4+dcDC4qHzX4gRrtA4yAhExDzot0iq9anvg1w@mail.gmail.com> (Matthias Meulien's message of "Fri, 9 Dec 2011 17:28:50 +0100")
[-- Attachment #1: Type: text/plain, Size: 4927 bytes --]
Matthias Meulien <orontee@gmail.com> 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.
[-- Attachment #2: Matthias Meulien's patch to modernize bookmark menu header display, with some revisions by Karl Fogel. --]
[-- Type: text/plain, Size: 3547 bytes --]
=== 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))))
next parent reply other threads:[~2011-12-17 21:36 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAFEQCfBwuU54R4+dcDC4qHzX4gRrtA4yAhExDzot0iq9anvg1w@mail.gmail.com>
2011-12-17 21:36 ` Karl Fogel [this message]
2011-12-17 22:22 ` Patch to bookmark.el Stefan Monnier
2011-12-17 22:23 ` Karl Fogel
2011-12-19 19:30 ` Glenn Morris
2011-12-19 21:16 ` Matthias Meulien
2011-12-19 21:25 ` Glenn Morris
2011-12-18 5:30 ` Karl Fogel
2011-12-19 21:14 ` Matthias Meulien
2011-12-20 18:11 ` Karl Fogel
2011-12-20 23:03 ` Matthias Meulien
2011-12-20 23:29 ` Karl Fogel
2013-03-07 15:37 ` Matthias Meulien
2013-03-07 22:37 ` Karl Fogel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zkeqhod2.fsf@floss.red-bean.com \
--to=kfogel@red-bean.com \
--cc=emacs-devel@gnu.org \
--cc=orontee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.