unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#1183: 23.0.60; ediff-buffers is broken
@ 2008-10-16 18:47 ` Drew Adams
  2008-10-16 20:25   ` Eli Zaretskii
  2008-10-19 15:40   ` bug#1183: marked as done (23.0.60; ediff-buffers is broken) Emacs bug Tracking System
  0 siblings, 2 replies; 27+ messages in thread
From: Drew Adams @ 2008-10-16 18:47 UTC (permalink / raw)
  To: emacs-pretest-bug

emacs -Q
 
Visit both the bookmark.el file from this installation (see below) and
a copy of bookmark.el from CVS for today, 2008-10-16.
 
The only difference between the two files is in fact this text which
was added to the CVS version near the end of the file, just before
run-hooks:
 
(defun bookmark-unload-function ()
  "Unload the Bookmark library."
  (when bookmark-save-flag (bookmark-save))
  ;; continue standard unloading
  nil)
 
You can see this by using ediff in Emacs 22, 21, or 20 - or by using
diff.
 
However, ediff-buffers shows the entire buffers as a single diff, and
hitting `*' to refine that diff has no effect at all. IOW,
ediff-buffers is now useless for seeing the differences between these
two files.
 

In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
 of 2008-10-03 on LENNART-69DE564
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --no-opt --cflags -Ic:/g/include
-fno-crossjumping'
 







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-16 18:47 ` bug#1183: 23.0.60; ediff-buffers is broken Drew Adams
@ 2008-10-16 20:25   ` Eli Zaretskii
  2008-10-16 20:45     ` Drew Adams
  2008-10-19 15:40   ` bug#1183: marked as done (23.0.60; ediff-buffers is broken) Emacs bug Tracking System
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-16 20:25 UTC (permalink / raw)
  To: Drew Adams, 1183; +Cc: emacs-pretest-bug, bug-gnu-emacs

> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Thu, 16 Oct 2008 11:47:11 -0700
> Cc: 
> 
> emacs -Q
>  
> Visit both the bookmark.el file from this installation (see below) and
> a copy of bookmark.el from CVS for today, 2008-10-16.
>  
> The only difference between the two files is in fact this text which
> was added to the CVS version near the end of the file, just before
> run-hooks:
>  
> (defun bookmark-unload-function ()
>   "Unload the Bookmark library."
>   (when bookmark-save-flag (bookmark-save))
>   ;; continue standard unloading
>   nil)
>  
> You can see this by using ediff in Emacs 22, 21, or 20 - or by using
> diff.
>  
> However, ediff-buffers shows the entire buffers as a single diff, and
> hitting `*' to refine that diff has no effect at all. IOW,
> ediff-buffers is now useless for seeing the differences between these
> two files.

I don't have the ``bookmark.el file from this installation'' (you
didn't attach it), so I produced it manually by copying today's
bookmark.el and then removing the function bookmark-unload-function.

With these two files, I can only reproduce this with Emacs built on
Windows from today's CVS if one of the files has Unix end-of-line
format, while the other has DOS/Windows EOL format.  Is that your
case?

If so, this is expected: Ediff on Windows invokes the `diff' program
with the --binary option (see ediff-diff-options), which makes all
lines compare not equal due to the different line endings.  You can
see this by using `diff' directly from the shell's prompt, if you pass
it the --binary option.

If both files have identical EOL format, Ediff produces the output
you'd expect.

(The reason for the --binary option is to allow comparison of buffers
and files with non-ASCII text, which IMO is a much more important
use-case than two almost identical files with different line endings.)






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-16 20:25   ` Eli Zaretskii
@ 2008-10-16 20:45     ` Drew Adams
  2008-10-16 21:15       ` Eli Zaretskii
  0 siblings, 1 reply; 27+ messages in thread
From: Drew Adams @ 2008-10-16 20:45 UTC (permalink / raw)
  To: 'Eli Zaretskii', 1183; +Cc: emacs-pretest-bug, bug-gnu-emacs

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

> From: Eli Zaretskii Sent: Thursday, October 16, 2008 1:25 PM
> > From: "Drew Adams" <drew.adams@oracle.com>
> > emacs -Q
> >  
> > Visit both the bookmark.el file from this installation (see 
> > below) and a copy of bookmark.el from CVS for today, 2008-10-16.
> >  
> > The only difference between the two files is in fact this text which
> > was added to the CVS version near the end of the file, just before
> > run-hooks:
> >  
> > (defun bookmark-unload-function ()
> >   "Unload the Bookmark library."
> >   (when bookmark-save-flag (bookmark-save))
> >   ;; continue standard unloading
> >   nil)
> >  
> > You can see this by using ediff in Emacs 22, 21, or 20 - or by using
> > diff.
> >  
> > However, ediff-buffers shows the entire buffers as a single 
> > diff, and hitting `*' to refine that diff has no effect at all. IOW,
> > ediff-buffers is now useless for seeing the differences 
> > between these two files.
> 
> I don't have the ``bookmark.el file from this installation'' (you
> didn't attach it),

Attached now. I didn't think I had to attach it since I referenced its build -
sorry.

> so I produced it manually by copying today's
> bookmark.el and then removing the function bookmark-unload-function.

Same diff. ;-)

> With these two files, I can only reproduce this with Emacs built on
> Windows from today's CVS if one of the files has Unix end-of-line
> format, while the other has DOS/Windows EOL format.  Is that your
> case?

I downloaded the CVS version (with Unix format) from
http://cvs.savannah.gnu.org/viewvc/emacs/emacs/lisp/bookmark.el?view=log by
clicking the first "download" link on the page. The other file was in the
distribution.

Yes, I guess it is my case: The CVS file has "(Unix)" in the mode line; the
installation file has nothing.

> If so, this is expected:

Well it certainly isn't expected in Emacs 20, 21, or 22, where ediff-buffers
works perfectly for these two files. Why this change for Emacs 23? What's the
gain?

And how to change the buffers (e.g. line endings) so ediff-buffers will compare
them correctly? And why wouldn't ediff-buffers itself do that automatically
(perhaps after user confirmation)?

> Ediff on Windows invokes the `diff' program
> with the --binary option (see ediff-diff-options), which makes all
> lines compare not equal due to the different line endings.

Not very useful in situations like this. Comparing source code files where the
only difference is line endings should not produce a totally unusable diff: a
single, whole-file diff for which `*' has no effect (cannot be refined). 

It should show just the line endings as differences, if they are the only
differences. And in this case, where several text lines were different, it
should pick those lines up as a difference. Definitely. 

To me, this seems horribly broken.

> You can see this by using `diff' directly from the shell's prompt,
> if you pass it the --binary option.
> 
> If both files have identical EOL format, Ediff produces the output
> you'd expect.

And how to get that identical EOL format (I should know that, but I never bother
to change the format).

And why shouldn't ediff at least pick that up and let you know how to get to a
useful diff. Better yet, it should do that for you, perhaps after asking for
confirmation to change line endings or whatever.

What was wrong with the Emacs 22, 21, 20,... behavior?

> (The reason for the --binary option is to allow comparison of buffers
> and files with non-ASCII text, which IMO is a much more important
> use-case than two almost identical files with different line endings.)

Apples and oranges. They are probably both important.

Certainly it's important to be able to diff a file in the installation against a
CVS version of the file in a simple way.

Line-ending differences are not the most important differences to show, and in
fact even those differences are not apparent. You cannot tell anything at all
from the ediff-buffers diff - it tells you nothing about the differences except
that there are differences. Pretty useless.

I can't believe that one would argue that this behavior represents progress, at
least in its present form. Something should be done to fix this, IMO.


[-- Attachment #2: bookmark.el --]
[-- Type: application/octet-stream, Size: 79734 bytes --]

;;; bookmark.el --- set bookmarks, maybe annotate them, jump to them later

;; Copyright (C) 1993, 1994, 1995, 1996, 1997, 2001, 2002, 2003,
;;   2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.

;; Author: Karl Fogel <kfogel@red-bean.com>
;; Maintainer: Karl Fogel <kfogel@red-bean.com>
;; Created: July, 1993
;; Keywords: bookmarks, placeholders, annotations

;; This file is part of GNU Emacs.

;; GNU Emacs is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; GNU Emacs is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.

;;; Commentary:

;; This package is for setting "bookmarks" in files.  A bookmark
;; associates a string with a location in a certain file.  Thus, you
;; can navigate your way to that location by providing the string.
;; See the "User Variables" section for customizations.

;; Thanks to David Bremner <bremner@cs.sfu.ca> for thinking of and
;; then implementing the bookmark-current-bookmark idea.  He even
;; sent *patches*, bless his soul...

;; Thanks to Gregory M. Saunders <saunders@cis.ohio-state.edu> for
;; fixing and improving bookmark-time-to-save-p.

;; Thanks go to Andrew V. Klein <avk@cig.mot.com> for the code that
;; sorts the alist before presenting it to the user (in bookmark-bmenu-list
;; and the menu-bar).

;; And much thanks to David Hughes <djh@harston.cv.com> for many small
;; suggestions and the code to implement them (like
;; bookmark-bmenu-check-position, and some of the Lucid compatibility
;; stuff).

;; Kudos (whatever they are) go to Jim Blandy <jimb@red-bean.com>
;; for his eminently sensible suggestion to separate bookmark-jump
;; into bookmark-jump and bookmark-jump-noselect, which made many
;; other things cleaner as well.

;; Thanks to Roland McGrath for encouragement and help with defining
;; autoloads on the menu-bar.

;; Jonathan Stigelman <stig@hackvan.com> gave patches for default
;; values in bookmark-jump and bookmark-set.  Everybody please keep
;; all the keystrokes they save thereby and send them to him at the
;; end of each year :-)  (No, seriously, thanks Jonathan!)

;; Buckets of gratitude to John Grabowski <johng@media.mit.edu> for
;; thinking up the annotations feature and implementing it so well.

;; Based on info-bookmark.el, by Karl Fogel and Ken Olstad
;; <olstad@msc.edu>.

;; Thanks to Mikio Nakajima <PBC01764@niftyserve.or.jp> for many bugs
;; reported and fixed.

;; Thank you, Michael Kifer, for contributing the XEmacs support.

;; Enough with the credits already, get on to the good stuff:

;; FAVORITE CHINESE RESTAURANT:
;; Boy, that's a tough one.  Probably Hong Min, or maybe Emperor's
;; Choice (both in Chicago's Chinatown).  Well, both.  How about you?
\f
;;; Code:

(require 'pp)

;;; Misc comments:
;;
;; If variable bookmark-use-annotations is non-nil, an annotation is
;; queried for when setting a bookmark.
;;
;; The bookmark list is sorted lexically by default, but you can turn
;; this off by setting bookmark-sort-flag to nil.  If it is nil, then
;; the list will be presented in the order it is recorded
;; (chronologically), which is actually fairly useful as well.

;;; User Variables

(defgroup bookmark nil
  "Setting, annotation and jumping to bookmarks."
  :group 'matching)


(defcustom bookmark-use-annotations nil
  "If non-nil, saving a bookmark queries for an annotation in a buffer."
  :type 'boolean
  :group 'bookmark)


(defcustom bookmark-save-flag t
  "Controls when Emacs saves bookmarks to a file.
--> nil means never save bookmarks, except when `bookmark-save' is
    explicitly called \(\\[bookmark-save]\).
--> t means save bookmarks when Emacs is killed.
--> Otherwise, it should be a number that is the frequency with which
    the bookmark list is saved \(i.e.: the number of times which
    Emacs' bookmark list may be modified before it is automatically
    saved.\).  If it is a number, Emacs will also automatically save
    bookmarks when it is killed.

Therefore, the way to get it to save every time you make or delete a
bookmark is to set this variable to 1 \(or 0, which produces the same
behavior.\)

To specify the file in which to save them, modify the variable
`bookmark-default-file', which is `~/.emacs.bmk' by default."
  :type '(choice (const nil) integer (other t))
  :group 'bookmark)


(defconst bookmark-old-default-file "~/.emacs-bkmrks"
  "*The `.emacs.bmk' file used to be called this name.")


;; defvarred to avoid a compilation warning:
(defvar bookmark-file nil
  "Old name for `bookmark-default-file'.")

(defcustom bookmark-default-file
  (if bookmark-file
      ;; In case user set `bookmark-file' in her .emacs:
      bookmark-file
    (convert-standard-filename "~/.emacs.bmk"))
  "File in which to save bookmarks by default."
  :type 'file
  :group 'bookmark)


(defcustom bookmark-version-control 'nospecial
  "Whether or not to make numbered backups of the bookmark file.
It can have four values: t, nil, `never', and `nospecial'.
The first three have the same meaning that they do for the
variable `version-control', and the final value `nospecial' means just
use the value of `version-control'."
  :type '(choice (const nil) (const never) (const nospecial)
		 (other t))
  :group 'bookmark)


(defcustom bookmark-completion-ignore-case t
  "Non-nil means bookmark functions ignore case in completion."
  :type 'boolean
  :group 'bookmark)


(defcustom bookmark-sort-flag t
  "Non-nil means that bookmarks will be displayed sorted by bookmark name.
Otherwise they will be displayed in LIFO order (that is, most
recently set ones come first, oldest ones come last)."
  :type 'boolean
  :group 'bookmark)


(defcustom bookmark-automatically-show-annotations t
  "Non-nil means show annotations when jumping to a bookmark."
  :type 'boolean
  :group 'bookmark)


(defcustom bookmark-bmenu-file-column 30
  "Column at which to display filenames in a buffer listing bookmarks.
You can toggle whether files are shown with \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-toggle-filenames]."
  :type 'integer
  :group 'bookmark)


(defcustom bookmark-bmenu-toggle-filenames t
  "Non-nil means show filenames when listing bookmarks.
This may result in truncated bookmark names.  To disable this, put the
following in your `.emacs' file:

\(setq bookmark-bmenu-toggle-filenames nil\)"
  :type 'boolean
  :group 'bookmark)


(defcustom bookmark-menu-length 70
  "Maximum length of a bookmark name displayed on a popup menu."
  :type 'integer
  :group 'bookmark)


(defface bookmark-menu-heading
  '((t (:inherit font-lock-type-face)))
  "Face used to highlight the heading in bookmark menu buffers."
  :group 'bookmark
  :version "22.1")


;;; No user-serviceable parts beyond this point.

;; Added  for lucid emacs  compatibility, db
(or (fboundp 'defalias)  (fset 'defalias 'fset))

;; suggested for lucid compatibility by david hughes:
(or (fboundp 'frame-height)  (defalias 'frame-height 'screen-height))

\f
;;; Keymap stuff:

;; Set up these bindings dumping time *only*;
;; if the user alters them, don't override the user when loading bookmark.el.

;;;###autoload (define-key ctl-x-r-map "b" 'bookmark-jump)
;;;###autoload (define-key ctl-x-r-map "m" 'bookmark-set)
;;;###autoload (define-key ctl-x-r-map "l" 'bookmark-bmenu-list)

;;;###autoload
(defvar bookmark-map
  (let ((map (make-sparse-keymap)))
    ;; Read the help on all of these functions for details...
    (define-key map "x" 'bookmark-set)
    (define-key map "m" 'bookmark-set) ;"m"ark
    (define-key map "j" 'bookmark-jump)
    (define-key map "g" 'bookmark-jump) ;"g"o
    (define-key map "o" 'bookmark-jump-other-window)
    (define-key map "i" 'bookmark-insert)
    (define-key map "e" 'edit-bookmarks)
    (define-key map "f" 'bookmark-insert-location) ;"f"ind
    (define-key map "r" 'bookmark-rename)
    (define-key map "d" 'bookmark-delete)
    (define-key map "l" 'bookmark-load)
    (define-key map "w" 'bookmark-write)
    (define-key map "s" 'bookmark-save)
    map)
  "Keymap containing bindings to bookmark functions.
It is not bound to any key by default: to bind it
so that you have a bookmark prefix, just use `global-set-key' and bind a
key of your choice to `bookmark-map'.  All interactive bookmark
functions have a binding in this keymap.")

;;;###autoload (fset 'bookmark-map bookmark-map)

\f
;;; Core variables and data structures:
(defvar bookmark-alist ()
  "Association list of bookmarks and their records.
You probably don't want to change the value of this alist yourself;
instead, let the various bookmark functions do it for you.

The format of the alist is

       \(BOOKMARK1 BOOKMARK2 ...\)

where each BOOKMARK is of the form

  (NAME PARAM-ALIST) or (NAME . PARAM-ALIST)

where the first form is the old deprecated one and the second is
the new favored one.  PARAM-ALIST is typically of the form:

 ((filename . FILE)
  (front-context-string . FRONT-STR)
  (rear-context-string  . REAR-STR)
  (position . POS)
  (annotation . ANNOTATION)))")


(defvar bookmarks-already-loaded nil)


;; more stuff added by db.

(defvar bookmark-current-bookmark nil
  "Name of bookmark most recently used in the current file.
It is buffer local, used to make moving a bookmark forward
through a file easier.")

(make-variable-buffer-local 'bookmark-current-bookmark)


(defvar bookmark-alist-modification-count 0
  "Number of modifications to bookmark list since it was last saved.")


(defvar bookmark-search-size 16
  "Length of the context strings recorded on either side of a bookmark.")


(defvar bookmark-current-point 0)
(defvar bookmark-yank-point 0)
(defvar bookmark-current-buffer nil)

(defvar Info-suffix-list)
\f
;; Helper functions.

;; Only functions on this page and the next one (file formats) need to
;; know anything about the format of bookmark-alist entries.
;; Everyone else should go through them.


(defun bookmark-name-from-full-record (full-record)
  "Return name of FULL-RECORD \(an alist element instead of a string\)."
  (car full-record))


(defun bookmark-all-names ()
  "Return a list of all current bookmark names."
  (bookmark-maybe-load-default-file)
  (mapcar 'bookmark-name-from-full-record bookmark-alist))


(defun bookmark-get-bookmark (bookmark)
  "Return the bookmark record corresponding to BOOKMARK.
If BOOKMARK is already a bookmark record, just return it,
Otherwise look for the corresponding bookmark in `bookmark-alist'."
  (cond
   ((consp bookmark) bookmark)
   ((stringp bookmark)
    (assoc-string bookmark bookmark-alist bookmark-completion-ignore-case))))


(defun bookmark-get-bookmark-record (bookmark)
  "Return the guts of the entry for BOOKMARK in `bookmark-alist'.
That is, all information but the name."
  (let ((alist (cdr (bookmark-get-bookmark bookmark))))
    ;; The bookmark objects can either look like (NAME ALIST) or
    ;; (NAME . ALIST), so we have to distinguish the two here.
    (if (and (null (cdr alist)) (consp (caar alist)))
        (car alist) alist)))


(defun bookmark-set-name (bookmark newname)
  "Set BOOKMARK's name to NEWNAME."
  (setcar
   (if (stringp bookmark) (bookmark-get-bookmark bookmark) bookmark)
   newname))

(defun bookmark-prop-get (bookmark prop)
  "Return the property PROP of BOOKMARK, or nil if none."
  (cdr (assq prop (bookmark-get-bookmark-record bookmark))))

(defun bookmark-prop-set (bookmark prop val)
  "Set the property PROP of BOOKMARK to VAL."
  (let ((cell (assq prop (bookmark-get-bookmark-record bookmark))))
    (if cell
        (setcdr cell val)
      (nconc (bookmark-get-bookmark-record bookmark)
             (list (cons prop val))))))

(defun bookmark-get-annotation (bookmark)
  "Return the annotation of BOOKMARK, or nil if none."
  (bookmark-prop-get bookmark 'annotation))

(defun bookmark-set-annotation (bookmark ann)
  "Set the annotation of BOOKMARK to ANN."
  (bookmark-prop-set bookmark 'annotation ann))


(defun bookmark-get-filename (bookmark)
  "Return the full filename of BOOKMARK."
  (bookmark-prop-get bookmark 'filename))


(defun bookmark-set-filename (bookmark filename)
  "Set the full filename of BOOKMARK to FILENAME."
  (bookmark-prop-set bookmark 'filename filename))


(defun bookmark-get-position (bookmark)
  "Return the position \(i.e.: point\) of BOOKMARK."
  (bookmark-prop-get bookmark 'position))


(defun bookmark-set-position (bookmark position)
  "Set the position \(i.e.: point\) of BOOKMARK to POSITION."
  (bookmark-prop-set bookmark 'position position))


(defun bookmark-get-front-context-string (bookmark)
  "Return the front-context-string of BOOKMARK."
  (bookmark-prop-get bookmark 'front-context-string))


(defun bookmark-set-front-context-string (bookmark string)
  "Set the front-context-string of BOOKMARK to STRING."
  (bookmark-prop-set bookmark 'front-context-string string))


(defun bookmark-get-rear-context-string (bookmark)
  "Return the rear-context-string of BOOKMARK."
  (bookmark-prop-get bookmark 'rear-context-string))


(defun bookmark-set-rear-context-string (bookmark string)
  "Set the rear-context-string of BOOKMARK to STRING."
  (bookmark-prop-set bookmark 'rear-context-string string))


(defun bookmark-get-handler (bookmark)
  (bookmark-prop-get bookmark 'handler))

(defvar bookmark-history nil
  "The history list for bookmark functions.")


(defun bookmark-completing-read (prompt &optional default)
  "Prompting with PROMPT, read a bookmark name in completion.
PROMPT will get a \": \" stuck on the end no matter what, so you
probably don't want to include one yourself.
Optional second arg DEFAULT is a string to return if the user enters
the empty string."
  (bookmark-maybe-load-default-file) ; paranoia
  (if (listp last-nonmenu-event)
      (bookmark-menu-popup-paned-menu t prompt (bookmark-all-names))
    (let* ((completion-ignore-case bookmark-completion-ignore-case)
	   (default default)
	   (prompt (if default
		       (concat prompt (format " (%s): " default))
		     (concat prompt ": ")))
	   (str
	    (completing-read prompt
			     bookmark-alist
			     nil
			     0
			     nil
			     'bookmark-history)))
      (if (string-equal "" str) default str))))


(defmacro bookmark-maybe-historicize-string (string)
  "Put STRING into the bookmark prompt history, if caller non-interactive.
We need this because sometimes bookmark functions are invoked from
menus, so `completing-read' never gets a chance to set `bookmark-history'."
  `(or
    (interactive-p)
    (setq bookmark-history (cons ,string bookmark-history))))

(defvar bookmark-make-record-function 'bookmark-make-record-default
  "A function that should be called to create a bookmark record.
Modes may set this variable buffer-locally to enable bookmarking of
locations that should be treated specially, such as Info nodes,
news posts, images, pdf documents, etc.

The function will be called with no arguments.
It should signal a user error if it is unable to construct a record for
the current location.

The returned record should be a cons cell of the form (NAME . ALIST)
where ALIST is as described in `bookmark-alist' and may typically contain
a special cons (handler . SOME-FUNCTION) which sets the handler function
that should be used to open this bookmark instead of
`bookmark-default-handler'.  The handler should follow the same calling
convention as the one used by `bookmark-default-handler'.

NAME is a suggested name for the constructed bookmark.  It can be nil
in which case a default heuristic will be used.  The function can also
equivalently just return ALIST without NAME.")

(defun bookmark-make-record ()
  "Return a new bookmark record (NAME . ALIST) for the current location."
  (let ((record (funcall bookmark-make-record-function)))
    ;; Set up default name.
    (if (stringp (car record))
        ;; The function already provided a default name.
        record
      (if (car record) (push nil record))
      (setcar record (or bookmark-current-bookmark (bookmark-buffer-name)))
      record)))

(defun bookmark-store (name alist no-overwrite)
  "Store the bookmark NAME with data ALIST.
If NO-OVERWRITE is non-nil and another bookmark of the same name already
exists in `bookmark-alist', record the new bookmark without throwing away the
old one."
  (bookmark-maybe-load-default-file)
  (let ((stripped-name (copy-sequence name)))
    (or (featurep 'xemacs)
        ;; XEmacs's `set-text-properties' doesn't work on
        ;; free-standing strings, apparently.
        (set-text-properties 0 (length stripped-name) nil stripped-name))
    (if (and (bookmark-get-bookmark stripped-name) (not no-overwrite))
        ;; already existing bookmark under that name and
        ;; no prefix arg means just overwrite old bookmark
        ;; Use the new (NAME . ALIST) format.
        (setcdr (bookmark-get-bookmark stripped-name) alist)

      ;; otherwise just cons it onto the front (either the bookmark
      ;; doesn't exist already, or there is no prefix arg.  In either
      ;; case, we want the new bookmark consed onto the alist...)

      (push (cons stripped-name alist) bookmark-alist))

    ;; Added by db
    (setq bookmark-current-bookmark stripped-name)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))

    (setq bookmark-current-bookmark stripped-name)
    (bookmark-bmenu-surreptitiously-rebuild-list)))

(defun bookmark-make-record-default (&optional point-only)
  "Return the record describing the location of a new bookmark.
Must be at the correct position in the buffer in which the bookmark is
being set.
If POINT-ONLY is non-nil, then only return the subset of the
record that pertains to the location within the buffer."
  `(,@(unless point-only `((filename . ,(bookmark-buffer-file-name))))
    (front-context-string
     . ,(if (>= (- (point-max) (point)) bookmark-search-size)
            (buffer-substring-no-properties
             (point)
             (+ (point) bookmark-search-size))
          nil))
    (rear-context-string
     . ,(if (>= (- (point) (point-min)) bookmark-search-size)
            (buffer-substring-no-properties
             (point)
             (- (point) bookmark-search-size))
          nil))
    (position . ,(point))))

\f
;;; File format stuff

;; The OLD format of the bookmark-alist was:
;;
;;       ((BOOKMARK-NAME . (FILENAME
;;                          STRING-IN-FRONT
;;                          STRING-BEHIND
;;                          POINT))
;;        ...)
;;
;; The NEW format of the bookmark-alist is:
;;
;;       ((BOOKMARK-NAME (filename   . FILENAME)
;;                       (front-context-string . STRING-IN-FRONT)
;;                       (rear-context-string  . STRING-BEHIND)
;;                       (position   . POINT)
;;                       (annotation . ANNOTATION)
;;                       (whatever   . VALUE)
;;                       ...
;;                       ))
;;        ...)
;;
;;
;; I switched to using an internal as well as external alist because I
;; felt that would be a more flexible framework in which to add
;; features.  It means that the order in which values appear doesn't
;; matter, and it means that arbitrary values can be added without
;; risk of interfering with existing ones.
;;
;; BOOKMARK-NAME is the string the user gives the bookmark and
;; accesses it by from then on.
;;
;; FILENAME is the location of the file in which the bookmark is set.
;;
;; STRING-IN-FRONT is a string of `bookmark-search-size' chars of
;; context in front of the point at which the bookmark is set.
;;
;; STRING-BEHIND is the same thing, but after the point.
;;
;; The context strings exist so that modifications to a file don't
;; necessarily cause a bookmark's position to be invalidated.
;; bookmark-jump will search for STRING-BEHIND and STRING-IN-FRONT in
;; case the file has changed since the bookmark was set.  It will
;; attempt to place the user before the changes, if there were any.
;; ANNOTATION is the annotation for the bookmark; it may not exist
;; (for backward compatibility), be nil (no annotation), or be a
;; string.


(defconst bookmark-file-format-version 1
  "The current version of the format used by bookmark files.
You should never need to change this.")


(defconst bookmark-end-of-version-stamp-marker
  "-*- End Of Bookmark File Format Version Stamp -*-\n"
  "This string marks the end of the version stamp in a bookmark file.")


(defun bookmark-alist-from-buffer ()
  "Return a `bookmark-alist' (in any format) from the current buffer.
The buffer must of course contain bookmark format information.
Does not care from where in the buffer it is called, and does not
affect point."
  (save-excursion
    (goto-char (point-min))
    (if (search-forward bookmark-end-of-version-stamp-marker nil t)
        (read (current-buffer))
      ;; Else we're dealing with format version 0
      (if (search-forward "(" nil t)
          (progn
            (forward-char -1)
            (read (current-buffer)))
        ;; Else no hope of getting information here.
        (error "Not bookmark format")))))


(defun bookmark-upgrade-version-0-alist (old-list)
  "Upgrade a version 0 alist OLD-LIST to the current version."
  (mapcar
   (lambda (bookmark)
     (let* ((name      (car bookmark))
            (record    (car (cdr bookmark)))
            (filename  (nth 0 record))
            (front-str (nth 1 record))
            (rear-str  (nth 2 record))
            (position  (nth 3 record))
            (ann       (nth 4 record)))
       (list
        name
        `((filename             .    ,filename)
          (front-context-string .    ,(or front-str ""))
          (rear-context-string  .    ,(or rear-str  ""))
          (position             .    ,position)
          (annotation           .    ,ann)))))
   old-list))


(defun bookmark-upgrade-file-format-from-0 ()
  "Upgrade a bookmark file of format 0 (the original format) to format 1.
This expects to be called from `point-min' in a bookmark file."
  (message "Upgrading bookmark format from 0 to %d..."
           bookmark-file-format-version)
  (let* ((old-list (bookmark-alist-from-buffer))
         (new-list (bookmark-upgrade-version-0-alist old-list)))
    (delete-region (point-min) (point-max))
    (bookmark-insert-file-format-version-stamp)
    (pp new-list (current-buffer))
    (save-buffer))
  (goto-char (point-min))
  (message "Upgrading bookmark format from 0 to %d...done"
           bookmark-file-format-version)
  )


(defun bookmark-grok-file-format-version ()
  "Return an integer which is the file-format version of this bookmark file.
This expects to be called from `point-min' in a bookmark file."
  (if (looking-at "^;;;;")
      (save-excursion
        (save-match-data
          (re-search-forward "[0-9]")
          (forward-char -1)
          (read (current-buffer))))
    ;; Else this is format version 0, the original one, which didn't
    ;; even have version stamps.
    0))


(defun bookmark-maybe-upgrade-file-format ()
  "Check the file-format version of this bookmark file.
If the version is not up-to-date, upgrade it automatically.
This expects to be called from `point-min' in a bookmark file."
  (let ((version (bookmark-grok-file-format-version)))
    (cond
     ((= version bookmark-file-format-version)
      ) ; home free -- version is current
     ((= version 0)
      (bookmark-upgrade-file-format-from-0))
     (t
      (error "Bookmark file format version strangeness")))))


(defun bookmark-insert-file-format-version-stamp ()
  "Insert text indicating current version of bookmark file format."
  (insert
   (format ";;;; Emacs Bookmark Format Version %d ;;;;\n"
           bookmark-file-format-version))
  (insert ";;; This format is meant to be slightly human-readable;\n"
          ";;; nevertheless, you probably don't want to edit it.\n"
          ";;; "
          bookmark-end-of-version-stamp-marker))


;;; end file-format stuff

\f
;;; Generic helpers.

(defun bookmark-maybe-message (fmt &rest args)
  "Apply `message' to FMT and ARGS, but only if the display is fast enough."
  (if (>= baud-rate 9600)
      (apply 'message fmt args)))

\f
;;; Core code:

(defvar bookmark-minibuffer-read-name-map
  (let ((map (make-sparse-keymap)))
    (set-keymap-parent map minibuffer-local-map)
    (define-key map "\C-w" 'bookmark-yank-word)
    ;; This C-u binding might not be very useful any more now that we
    ;; provide access to the default via the standard M-n binding.
    ;; Maybe we should just remove it?  --Stef-08
    (define-key map "\C-u" 'bookmark-insert-current-bookmark)
    map))

;;;###autoload
(defun bookmark-set (&optional name parg)
  "Set a bookmark named NAME inside a file.
If name is nil, then the user will be prompted.
With prefix arg, will not overwrite a bookmark that has the same name
as NAME if such a bookmark already exists, but instead will \"push\"
the new bookmark onto the bookmark alist.  Thus the most recently set
bookmark with name NAME would be the one in effect at any given time,
but the others are still there, should you decide to delete the most
recent one.

To yank words from the text of the buffer and use them as part of the
bookmark name, type C-w while setting a bookmark.  Successive C-w's
yank successive words.

Typing C-u inserts the name of the last bookmark used in the buffer
\(as an aid in using a single bookmark name to track your progress
through a large file\).  If no bookmark was used, then C-u inserts the
name of the file being visited.

Use \\[bookmark-delete] to remove bookmarks \(you give it a name,
and it removes only the first instance of a bookmark with that name from
the list of bookmarks.\)"
  (interactive (list nil current-prefix-arg))
  (let* ((record (bookmark-make-record))
         (default (car record)))

    (bookmark-maybe-load-default-file)

    (setq bookmark-current-point (point))
    (setq bookmark-yank-point (point))
    (setq bookmark-current-buffer (current-buffer))

    (let ((str
           (or name
               (read-from-minibuffer
                (format "Set bookmark (%s): " default)
                nil
                bookmark-minibuffer-read-name-map
                nil nil default))))
      (and (string-equal str "") (setq str default))
      (bookmark-store str (cdr record) parg)
      
      ;; Ask for an annotation buffer for this bookmark
      (if bookmark-use-annotations
          (bookmark-edit-annotation str)
        (goto-char bookmark-current-point)))))

(defun bookmark-kill-line (&optional newline-too)
  "Kill from point to end of line.
If optional arg NEWLINE-TOO is non-nil, delete the newline too.
Does not affect the kill ring."
  (let ((eol (save-excursion (end-of-line) (point))))
    (delete-region (point) eol)
    (if (and newline-too (looking-at "\n"))
        (delete-char 1))))


;; Defvars to avoid compilation warnings:
(defvar bookmark-annotation-name nil
  "Variable holding the name of the bookmark.
This is used in `bookmark-edit-annotation' to record the bookmark
whose annotation is being edited.")


(defun bookmark-default-annotation-text (bookmark)
  (concat "#  Type the annotation for bookmark '" bookmark "' here.\n"
	  "#  All lines which start with a '#' will be deleted.\n"
	  "#  Type C-c C-c when done.\n#\n"
	  "#  Author: " (user-full-name) " <" (user-login-name) "@"
	  (system-name) ">\n"
	  "#  Date:    " (current-time-string) "\n"))


(defvar bookmark-edit-annotation-text-func 'bookmark-default-annotation-text
  "Function to return default text to use for a bookmark annotation.
It takes one argument, the name of the bookmark, as a string.")
(define-obsolete-variable-alias 'bookmark-read-annotation-text-func
  'bookmark-edit-annotation-text-func "23.1")

(defvar bookmark-edit-annotation-mode-map
  (let ((map (make-sparse-keymap)))
    (set-keymap-parent map text-mode-map)
    (define-key map "\C-c\C-c" 'bookmark-send-edited-annotation)
    map)
  "Keymap for editing an annotation of a bookmark.")


(defun bookmark-edit-annotation-mode (bookmark)
  "Mode for editing the annotation of bookmark BOOKMARK.
When you have finished composing, type \\[bookmark-send-annotation].

\\{bookmark-edit-annotation-mode-map}"
  (interactive)
  (kill-all-local-variables)
  (make-local-variable 'bookmark-annotation-name)
  (setq bookmark-annotation-name bookmark)
  (use-local-map bookmark-edit-annotation-mode-map)
  (setq major-mode 'bookmark-edit-annotation-mode
        mode-name "Edit Bookmark Annotation")
  (insert (funcall bookmark-edit-annotation-text-func bookmark))
  (let ((annotation (bookmark-get-annotation bookmark)))
    (if (and annotation (not (string-equal annotation "")))
	(insert annotation)))
  (run-mode-hooks 'text-mode-hook))


(defun bookmark-send-edited-annotation ()
  "Use buffer contents as annotation for a bookmark.
Lines beginning with `#' are ignored."
  (interactive)
  (if (not (eq major-mode 'bookmark-edit-annotation-mode))
      (error "Not in bookmark-edit-annotation-mode"))
  (goto-char (point-min))
  (while (< (point) (point-max))
    (if (looking-at "^#")
        (bookmark-kill-line t)
      (forward-line 1)))
  ;; Take no chances with text properties.
  (let ((annotation (buffer-substring-no-properties (point-min) (point-max)))
	(bookmark bookmark-annotation-name))
    (bookmark-set-annotation bookmark annotation)
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (goto-char bookmark-current-point))
  (kill-buffer (current-buffer)))


(defun bookmark-edit-annotation (bookmark)
  "Pop up a buffer for editing bookmark BOOKMARK's annotation."
  (pop-to-buffer (generate-new-buffer-name "*Bookmark Annotation Compose*"))
  (bookmark-edit-annotation-mode bookmark))


(defun bookmark-insert-current-bookmark ()
  "Insert this buffer's value of `bookmark-current-bookmark'.
Default to file name if it's nil."
  (interactive)
  (let ((str
	 (with-current-buffer bookmark-current-buffer
	   (or bookmark-current-bookmark
               (bookmark-buffer-name)))))
    (insert str)))


(defun bookmark-buffer-name ()
  "Return the name of the current buffer's file, non-directory."
  (cond
   ;; Or are we a file?
   (buffer-file-name (file-name-nondirectory buffer-file-name))
   ;; Or are we a directory?
   ((and (boundp 'dired-directory) dired-directory)
    (let* ((dirname (if (stringp dired-directory)
                        dired-directory
                      (car dired-directory)))
           (idx (1- (length dirname))))
      ;; Strip the trailing slash.
      (if (= ?/ (aref dirname idx))
          (file-name-nondirectory (substring dirname 0 idx))
        ;; Else return the current-buffer
        (buffer-name (current-buffer)))))
   ;; If all else fails, use the buffer's name.
   (t
    (buffer-name (current-buffer)))))


(defun bookmark-yank-word ()
  (interactive)
  ;; get the next word from the buffer and append it to the name of
  ;; the bookmark currently being set.
  (let ((string (with-current-buffer bookmark-current-buffer
                  (goto-char bookmark-yank-point)
                  (buffer-substring-no-properties
                   (point)
                   (progn
                     (forward-word 1)
                     (setq bookmark-yank-point (point)))))))
    (insert string)))

(defun bookmark-buffer-file-name ()
  "Return the current buffer's file in a way useful for bookmarks."
  (cond
   (buffer-file-name
    ;; Abbreviate the path, both so it's shorter and so it's more
    ;; portable.  E.g., the user's home dir might be a different
    ;; path on different machines, but "~/" will still reach it.
    (abbreviate-file-name buffer-file-name))
   ((and (boundp 'dired-directory) dired-directory)
    (if (stringp dired-directory)
        dired-directory
      (car dired-directory)))
   (t (error "Buffer not visiting a file or directory"))))


(defun bookmark-maybe-load-default-file ()
  (and (not bookmarks-already-loaded)
       (null bookmark-alist)
       (prog2
           (and
            ;; Possibly the old bookmark file, "~/.emacs-bkmrks", needs
            ;; to be renamed.
            (file-exists-p (expand-file-name bookmark-old-default-file))
            (not (file-exists-p (expand-file-name bookmark-default-file)))
            (rename-file (expand-file-name bookmark-old-default-file)
                         (expand-file-name bookmark-default-file)))
           ;; return t so the `and' will continue...
           t)

       (file-readable-p (expand-file-name bookmark-default-file))
       (bookmark-load bookmark-default-file t t)
       (setq bookmarks-already-loaded t)))


(defun bookmark-maybe-sort-alist ()
  ;;Return the bookmark-alist for display.  If the bookmark-sort-flag
  ;;is non-nil, then return a sorted copy of the alist.
  (if bookmark-sort-flag
      (sort (copy-alist bookmark-alist)
            (function
             (lambda (x y) (string-lessp (car x) (car y)))))
    bookmark-alist))


(defvar bookmark-after-jump-hook nil
  "Hook run after `bookmark-jump' jumps to a bookmark.
Useful for example to unhide text in `outline-mode'.")

(defun bookmark--jump-via (bookmark display-function)
  (bookmark-jump-noselect bookmark)
  (save-current-buffer
    (funcall display-function (current-buffer)))
  (let ((win (get-buffer-window (current-buffer) 0)))
    (if win (set-window-point win (point))))
  ;; FIXME: we used to only run bookmark-after-jump-hook in
  ;; `bookmark-jump' itself, but in none of the other commands.
  (run-hooks 'bookmark-after-jump-hook)
  (if bookmark-automatically-show-annotations
      ;; if there is an annotation for this bookmark,
      ;; show it in a buffer.
      (bookmark-show-annotation bookmark)))
  

;;;###autoload
(defun bookmark-jump (bookmark)
  "Jump to bookmark BOOKMARK (a point in some file).
You may have a problem using this function if the value of variable
`bookmark-alist' is nil.  If that happens, you need to load in some
bookmarks.  See help on function `bookmark-load' for more about
this.

If the file pointed to by BOOKMARK no longer exists, you will be asked
if you wish to give the bookmark a new location, and `bookmark-jump'
will then jump to the new location, as well as recording it in place
of the old one in the permanent bookmark record."
  (interactive
   (list (bookmark-completing-read "Jump to bookmark"
				   bookmark-current-bookmark)))
  (unless bookmark
    (error "No bookmark specified"))
  (bookmark-maybe-historicize-string bookmark)
  (bookmark--jump-via bookmark 'switch-to-buffer))


;;;###autoload
(defun bookmark-jump-other-window (bookmark)
  "Jump to BOOKMARK (a point in some file) in another window.
See `bookmark-jump'."
  (interactive
   (let ((bkm (bookmark-completing-read "Jump to bookmark (in another window)"
                                        bookmark-current-bookmark)))
     (if (> emacs-major-version 21)
         (list bkm) bkm)))
  (when bookmark
    (bookmark-maybe-historicize-string bookmark)
    (bookmark--jump-via bookmark 'switch-to-buffer-other-window)))


(defun bookmark-file-or-variation-thereof (file)
  "Return FILE (a string) if it exists, or return a reasonable
variation of FILE if that exists.  Reasonable variations are checked
by appending suffixes defined in `Info-suffix-list'.  If cannot find FILE
nor a reasonable variation thereof, then still return FILE if it can
be retrieved from a VC backend, else return nil."
  (if (file-exists-p file)
      file
    (or
     (progn (require 'info)  ; ensure Info-suffix-list is bound
            (catch 'found
              (mapc (lambda (elt)
                      (let ((suffixed-file (concat file (car elt))))
                        (if (file-exists-p suffixed-file)
                            (throw 'found suffixed-file))))
                    Info-suffix-list)
              nil))
     ;; Last possibility: try VC
     (if (vc-backend file) file))))

(defun bookmark-jump-noselect (bookmark)
  "Call BOOKMARK's handler or `bookmark-default-handler' if it has none.
Changes current buffer and point and returns nil, or signals a `file-error'.
BOOKMARK can be a bookmark record used internally by some other
elisp package, or the name of a bookmark to be found in `bookmark-alist'."
  (condition-case err
      (funcall (or (bookmark-get-handler bookmark)
                   'bookmark-default-handler)
               (bookmark-get-bookmark bookmark))
    (file-error
     ;; We were unable to find the marked file, so ask if user wants to
     ;; relocate the bookmark, else remind them to consider deletion.
     (when (stringp bookmark)
       ;; `bookmark' can either be a bookmark name (found in
       ;; `bookmark-alist') or a bookmark object.  If it's an object, we
       ;; assume it's a bookmark used internally by some other package.
       (let ((file (bookmark-get-filename bookmark)))
         (when file        ;Don't know how to relocate if there's no `file'.
           (setq file (expand-file-name file))
           (ding)
           (if (y-or-n-p (concat (file-name-nondirectory file)
                                 " nonexistent.  Relocate \""
                                 bookmark
                                 "\"? "))
               (progn
                 (bookmark-relocate bookmark)
                 ;; Try again.
                 (funcall (or (bookmark-get-handler bookmark)
                              'bookmark-default-handler)
                          (bookmark-get-bookmark bookmark)))
             (message
              "Bookmark not relocated; consider removing it \(%s\)." bookmark)
             (signal (car err) (cdr err))))))))
  ;; Added by db.
  (when (stringp bookmark)
    (setq bookmark-current-bookmark bookmark))
  nil)

(defun bookmark-default-handler (bmk)
  "Default handler to jump to a particular bookmark location.
BMK is a bookmark record.
Changes current buffer and point and returns nil, or signals a `file-error'."
  (let* ((file                   (bookmark-get-filename bmk))
         (buf                    (bookmark-prop-get bmk 'buffer))
         (forward-str            (bookmark-get-front-context-string bmk))
         (behind-str             (bookmark-get-rear-context-string bmk))
         (place                  (bookmark-get-position bmk)))
    ;; FIXME: bookmark-file-or-variation-thereof was needed for Info files,
    ;; but now that Info bookmarks are handled elsewhere it seems that we
    ;; should be able to get rid of it.  --Stef
    (if (not (if buf (buffer-live-p buf)
               (setq file (bookmark-file-or-variation-thereof file))))
        (signal 'file-error
                `("Jumping to bookmark" "No such file or directory"
                  (bookmark-get-filename bmk)))
      (set-buffer (or buf (find-file-noselect file)))
      (if place (goto-char place))

      ;; Go searching forward first.  Then, if forward-str exists and
      ;; was found in the file, we can search backward for behind-str.
      ;; Rationale is that if text was inserted between the two in the
      ;; file, it's better to be put before it so you can read it,
      ;; rather than after and remain perhaps unaware of the changes.
      (if forward-str
          (if (search-forward forward-str (point-max) t)
              (goto-char (match-beginning 0))))
      (if behind-str
          (if (search-backward behind-str (point-min) t)
              (goto-char (match-end 0)))))
    nil))

;;;###autoload
(defun bookmark-relocate (bookmark)
  "Relocate BOOKMARK to another file (reading file name with minibuffer).
This makes an already existing bookmark point to that file, instead of
the one it used to point at.  Useful when a file has been renamed
after a bookmark was set in it."
  (interactive (list (bookmark-completing-read "Bookmark to relocate")))
  (bookmark-maybe-historicize-string bookmark)
  (bookmark-maybe-load-default-file)
  (let* ((bmrk-filename (bookmark-get-filename bookmark))
         (newloc (expand-file-name
                  (read-file-name
                   (format "Relocate %s to: " bookmark)
                   (file-name-directory bmrk-filename)))))
    (bookmark-set-filename bookmark newloc)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))
    (bookmark-bmenu-surreptitiously-rebuild-list)))


;;;###autoload
(defun bookmark-insert-location (bookmark &optional no-history)
  "Insert the name of the file associated with BOOKMARK.
Optional second arg NO-HISTORY means don't record this in the
minibuffer history list `bookmark-history'."
  (interactive (list (bookmark-completing-read "Insert bookmark location")))
  (or no-history (bookmark-maybe-historicize-string bookmark))
  (let ((start (point)))
    (prog1
	(insert (bookmark-location bookmark)) ; *Return this line*
      (if (and (display-color-p) (display-mouse-p))
	  (add-text-properties
	   start
	   (save-excursion (re-search-backward
			    "[^ \t]")
					       (1+ (point)))
	   '(mouse-face highlight
	     follow-link t
	     help-echo "mouse-2: go to this bookmark in other window"))))))

;;;###autoload
(defalias 'bookmark-locate 'bookmark-insert-location)

(defun bookmark-location (bookmark)
  "Return the name of the file associated with BOOKMARK."
  (bookmark-maybe-load-default-file)
  (bookmark-get-filename bookmark))


;;;###autoload
(defun bookmark-rename (old &optional new)
  "Change the name of OLD bookmark to NEW name.
If called from keyboard, prompt for OLD and NEW.  If called from
menubar, select OLD from a menu and prompt for NEW.

If called from Lisp, prompt for NEW if only OLD was passed as an
argument.  If called with two strings, then no prompting is done.  You
must pass at least OLD when calling from Lisp.

While you are entering the new name, consecutive C-w's insert
consecutive words from the text of the buffer into the new bookmark
name."
  (interactive (list (bookmark-completing-read "Old bookmark name")))
  (bookmark-maybe-historicize-string old)
  (bookmark-maybe-load-default-file)

  (setq bookmark-current-point (point))
  (setq bookmark-yank-point (point))
  (setq bookmark-current-buffer (current-buffer))
  (let ((newname
         (or new   ; use second arg, if non-nil
             (read-from-minibuffer
              "New name: "
              nil
              (let ((now-map (copy-keymap minibuffer-local-map)))
                (define-key now-map  "\C-w" 'bookmark-yank-word)
                now-map)
              nil
              'bookmark-history))))
    (bookmark-set-name old newname)
    (setq bookmark-current-bookmark newname)
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))))


;;;###autoload
(defun bookmark-insert (bookmark)
  "Insert the text of the file pointed to by bookmark BOOKMARK.
You may have a problem using this function if the value of variable
`bookmark-alist' is nil.  If that happens, you need to load in some
bookmarks.  See help on function `bookmark-load' for more about
this."
  (interactive (list (bookmark-completing-read "Insert bookmark contents")))
  (bookmark-maybe-historicize-string bookmark)
  (bookmark-maybe-load-default-file)
  (let ((orig-point (point))
	(str-to-insert
	 (save-current-buffer
           (bookmark-jump-noselect bookmark)
	   (buffer-string))))
    (insert str-to-insert)
    (push-mark)
    (goto-char orig-point)))


;;;###autoload
(defun bookmark-delete (bookmark &optional batch)
  "Delete BOOKMARK from the bookmark list.
Removes only the first instance of a bookmark with that name.  If
there are one or more other bookmarks with the same name, they will
not be deleted.  Defaults to the \"current\" bookmark \(that is, the
one most recently used in this file, if any\).
Optional second arg BATCH means don't update the bookmark list buffer,
probably because we were called from there."
  (interactive
   (list (bookmark-completing-read "Delete bookmark"
				   bookmark-current-bookmark)))
  (bookmark-maybe-historicize-string bookmark)
  (bookmark-maybe-load-default-file)
  (let ((will-go (bookmark-get-bookmark bookmark)))
    (setq bookmark-alist (delq will-go bookmark-alist))
    ;; Added by db, nil bookmark-current-bookmark if the last
    ;; occurrence has been deleted
    (or (bookmark-get-bookmark bookmark-current-bookmark)
        (setq bookmark-current-bookmark nil)))
  ;; Don't rebuild the list
  (if batch
      nil
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))))


(defun bookmark-time-to-save-p (&optional last-time)
  ;; By Gregory M. Saunders <saunders@cis.ohio-state.edu>
  ;; finds out whether it's time to save bookmarks to a file, by
  ;; examining the value of variable bookmark-save-flag, and maybe
  ;; bookmark-alist-modification-count.  Returns t if they should be
  ;; saved, nil otherwise.  if last-time is non-nil, then this is
  ;; being called when emacs is killed.
  (cond (last-time
	 (and (> bookmark-alist-modification-count 0)
	      bookmark-save-flag))
	((numberp bookmark-save-flag)
	 (>= bookmark-alist-modification-count bookmark-save-flag))
	(t
	 nil)))


;;;###autoload
(defun bookmark-write ()
  "Write bookmarks to a file (reading the file name with the minibuffer).
Don't use this in Lisp programs; use `bookmark-save' instead."
  (interactive)
  (bookmark-maybe-load-default-file)
  (bookmark-save t))


;;;###autoload
(defun bookmark-save (&optional parg file)
  "Save currently defined bookmarks.
Saves by default in the file defined by the variable
`bookmark-default-file'.  With a prefix arg, save it in file FILE
\(second argument\).

If you are calling this from Lisp, the two arguments are PARG and
FILE, and if you just want it to write to the default file, then
pass no arguments.  Or pass in nil and FILE, and it will save in FILE
instead.  If you pass in one argument, and it is non-nil, then the
user will be interactively queried for a file to save in.

When you want to load in the bookmarks from a file, use
\`bookmark-load\', \\[bookmark-load].  That function will prompt you
for a file, defaulting to the file defined by variable
`bookmark-default-file'."
  (interactive "P")
  (bookmark-maybe-load-default-file)
  (cond
   ((and (null parg) (null file))
    ;;whether interactive or not, write to default file
    (bookmark-write-file bookmark-default-file))
   ((and (null parg) file)
    ;;whether interactive or not, write to given file
    (bookmark-write-file file))
   ((and parg (not file))
    ;;have been called interactively w/ prefix arg
    (let ((file (read-file-name "File to save bookmarks in: ")))
      (bookmark-write-file file)))
   (t ; someone called us with prefix-arg *and* a file, so just write to file
    (bookmark-write-file file)))
  ;; signal that we have synced the bookmark file by setting this to
  ;; 0.  If there was an error at any point before, it will not get
  ;; set, which is what we want.
  (setq bookmark-alist-modification-count 0))


\f
(defun bookmark-write-file (file)
  (bookmark-maybe-message "Saving bookmarks to file %s..." file)
  (with-current-buffer (get-buffer-create " *Bookmarks*")
    (goto-char (point-min))
    (delete-region (point-min) (point-max))
    (let ((print-length nil)
          (print-level nil))
      (bookmark-insert-file-format-version-stamp)
      (pp bookmark-alist (current-buffer))
      (let ((version-control
             (cond
              ((null bookmark-version-control) nil)
              ((eq 'never bookmark-version-control) 'never)
              ((eq 'nospecial bookmark-version-control) version-control)
              (t t))))
        (condition-case nil
            (write-region (point-min) (point-max) file)
          (file-error (message "Can't write %s" file)))
        (kill-buffer (current-buffer))
        (bookmark-maybe-message
         "Saving bookmarks to file %s...done" file)))))


(defun bookmark-import-new-list (new-list)
  ;; Walk over the new list, adding each individual bookmark
  ;; carefully.  "Carefully" means checking against the existing
  ;; bookmark-alist and renaming the new bookmarks with <N> extensions
  ;; as necessary.
  (let ((lst new-list)
        (names (bookmark-all-names)))
    (while lst
      (let* ((full-record (car lst)))
        (bookmark-maybe-rename full-record names)
        (setq bookmark-alist (nconc bookmark-alist (list full-record)))
        (setq names (cons (bookmark-name-from-full-record full-record) names))
        (setq lst (cdr lst))))))


(defun bookmark-maybe-rename (full-record names)
  ;; just a helper for bookmark-import-new-list; it is only for
  ;; readability that this is not inlined.
  ;;
  ;; Once this has found a free name, it sets full-record to that
  ;; name.
  (let ((found-name (bookmark-name-from-full-record full-record)))
    (if (member found-name names)
        ;; We've got a conflict, so generate a new name
        (let ((count 2)
              (new-name found-name))
          (while (member new-name names)
            (setq new-name (concat found-name (format "<%d>" count)))
            (setq count (1+ count)))
          (bookmark-set-name full-record new-name)))))


;;;###autoload
(defun bookmark-load (file &optional overwrite no-msg)
  "Load bookmarks from FILE (which must be in bookmark format).
Appends loaded bookmarks to the front of the list of bookmarks.  If
optional second argument OVERWRITE is non-nil, existing bookmarks are
destroyed.  Optional third arg NO-MSG means don't display any messages
while loading.

If you load a file that doesn't contain a proper bookmark alist, you
will corrupt Emacs's bookmark list.  Generally, you should only load
in files that were created with the bookmark functions in the first
place.  Your own personal bookmark file, `~/.emacs.bmk', is
maintained automatically by Emacs; you shouldn't need to load it
explicitly.

If you load a file containing bookmarks with the same names as
bookmarks already present in your Emacs, the new bookmarks will get
unique numeric suffixes \"<2>\", \"<3>\", ... following the same
method buffers use to resolve name collisions."
  (interactive
   (list (read-file-name
          (format "Load bookmarks from: (%s) "
                  bookmark-default-file)
          ;;Default might not be used often,
          ;;but there's no better default, and
          ;;I guess it's better than none at all.
          "~/" bookmark-default-file 'confirm)))
  (setq file (expand-file-name file))
  (if (not (file-readable-p file))
      (error "Cannot read bookmark file %s" file)
    (if (null no-msg)
        (bookmark-maybe-message "Loading bookmarks from %s..." file))
    (with-current-buffer (let ((enable-local-variables nil))
                           (find-file-noselect file))
      (goto-char (point-min))
      (bookmark-maybe-upgrade-file-format)
      (let ((blist (bookmark-alist-from-buffer)))
        (if (listp blist)
            (progn
              (if overwrite
                  (progn
                    (setq bookmark-alist blist)
                    (setq bookmark-alist-modification-count 0))
                ;; else
                (bookmark-import-new-list blist)
                (setq bookmark-alist-modification-count
                      (1+ bookmark-alist-modification-count)))
              (if (string-equal
                   (expand-file-name bookmark-default-file)
                   file)
                  (setq bookmarks-already-loaded t))
              (bookmark-bmenu-surreptitiously-rebuild-list))
          (error "Invalid bookmark list in %s" file)))
      (kill-buffer (current-buffer)))
    (if (null no-msg)
        (bookmark-maybe-message "Loading bookmarks from %s...done" file))))


\f
;;; Code supporting the dired-like bookmark menu.
;; Prefix is "bookmark-bmenu" for "buffer-menu":


(defvar bookmark-bmenu-bookmark-column nil)


(defvar bookmark-bmenu-hidden-bookmarks ())


(defvar bookmark-bmenu-mode-map nil)


(if bookmark-bmenu-mode-map
    nil
  (setq bookmark-bmenu-mode-map (make-keymap))
  (suppress-keymap bookmark-bmenu-mode-map t)
  (define-key bookmark-bmenu-mode-map "q" 'quit-window)
  (define-key bookmark-bmenu-mode-map "v" 'bookmark-bmenu-select)
  (define-key bookmark-bmenu-mode-map "w" 'bookmark-bmenu-locate)
  (define-key bookmark-bmenu-mode-map "2" 'bookmark-bmenu-2-window)
  (define-key bookmark-bmenu-mode-map "1" 'bookmark-bmenu-1-window)
  (define-key bookmark-bmenu-mode-map "j" 'bookmark-bmenu-this-window)
  (define-key bookmark-bmenu-mode-map "\C-c\C-c" 'bookmark-bmenu-this-window)
  (define-key bookmark-bmenu-mode-map "f" 'bookmark-bmenu-this-window)
  (define-key bookmark-bmenu-mode-map "\C-m" 'bookmark-bmenu-this-window)
  (define-key bookmark-bmenu-mode-map "o" 'bookmark-bmenu-other-window)
  (define-key bookmark-bmenu-mode-map "\C-o"
    'bookmark-bmenu-switch-other-window)
  (define-key bookmark-bmenu-mode-map "s" 'bookmark-bmenu-save)
  (define-key bookmark-bmenu-mode-map "k" 'bookmark-bmenu-delete)
  (define-key bookmark-bmenu-mode-map "\C-d" 'bookmark-bmenu-delete-backwards)
  (define-key bookmark-bmenu-mode-map "x" 'bookmark-bmenu-execute-deletions)
  (define-key bookmark-bmenu-mode-map "d" 'bookmark-bmenu-delete)
  (define-key bookmark-bmenu-mode-map " " 'next-line)
  (define-key bookmark-bmenu-mode-map "n" 'next-line)
  (define-key bookmark-bmenu-mode-map "p" 'previous-line)
  (define-key bookmark-bmenu-mode-map "\177" 'bookmark-bmenu-backup-unmark)
  (define-key bookmark-bmenu-mode-map "?" 'describe-mode)
  (define-key bookmark-bmenu-mode-map "u" 'bookmark-bmenu-unmark)
  (define-key bookmark-bmenu-mode-map "m" 'bookmark-bmenu-mark)
  (define-key bookmark-bmenu-mode-map "l" 'bookmark-bmenu-load)
  (define-key bookmark-bmenu-mode-map "r" 'bookmark-bmenu-rename)
  (define-key bookmark-bmenu-mode-map "R" 'bookmark-bmenu-relocate)
  (define-key bookmark-bmenu-mode-map "t" 'bookmark-bmenu-toggle-filenames)
  (define-key bookmark-bmenu-mode-map "a" 'bookmark-bmenu-show-annotation)
  (define-key bookmark-bmenu-mode-map "A" 'bookmark-bmenu-show-all-annotations)
  (define-key bookmark-bmenu-mode-map "e" 'bookmark-bmenu-edit-annotation)
  (define-key bookmark-bmenu-mode-map [mouse-2]
    'bookmark-bmenu-other-window-with-mouse))



;; Bookmark Buffer Menu mode is suitable only for specially formatted
;; data.
(put 'bookmark-bmenu-mode 'mode-class 'special)


;; todo: need to display whether or not bookmark exists as a buffer in
;; flag column.

;; Format:
;; FLAGS  BOOKMARK [ LOCATION ]


(defun bookmark-bmenu-surreptitiously-rebuild-list ()
  "Rebuild the Bookmark List if it exists.
Don't affect the buffer ring order."
  (if (get-buffer "*Bookmark List*")
      (save-excursion
        (save-window-excursion
          (bookmark-bmenu-list)))))


;;;###autoload
(defun bookmark-bmenu-list ()
  "Display a list of existing bookmarks.
The list is displayed in a buffer named `*Bookmark List*'.
The leftmost column displays a D if the bookmark is flagged for
deletion, or > if it is flagged for displaying."
  (interactive)
  (bookmark-maybe-load-default-file)
  (if (interactive-p)
      (switch-to-buffer (get-buffer-create "*Bookmark List*"))
    (set-buffer (get-buffer-create "*Bookmark List*")))
  (let ((inhibit-read-only t))
    (erase-buffer)
    (insert "% Bookmark\n- --------\n")
    (add-text-properties (point-min) (point)
			 '(font-lock-face bookmark-menu-heading))
    (mapc
     (lambda (full-record)
       ;; if a bookmark has an annotation, prepend a "*"
       ;; in the list of bookmarks.
       (let ((annotation (bookmark-get-annotation
                          (bookmark-name-from-full-record full-record))))
         (if (and annotation (not (string-equal annotation "")))
             (insert " *")
           (insert "  "))
	 (let ((start (point)))
	   (insert (bookmark-name-from-full-record full-record))
	   (if (and (display-color-p) (display-mouse-p))
	       (add-text-properties
		start
		(save-excursion (re-search-backward
				 "[^ \t]")
				(1+ (point)))
		'(mouse-face highlight
		  follow-link t
		  help-echo "mouse-2: go to this bookmark in other window")))
	   (insert "\n")
	   )))
     (bookmark-maybe-sort-alist)))
  (goto-char (point-min))
  (forward-line 2)
  (bookmark-bmenu-mode)
  (if bookmark-bmenu-toggle-filenames
      (bookmark-bmenu-toggle-filenames t)))

;;;###autoload
(defalias 'list-bookmarks 'bookmark-bmenu-list)
;;;###autoload
(defalias 'edit-bookmarks 'bookmark-bmenu-list)



(defun bookmark-bmenu-mode ()
  "Major mode for editing a list of bookmarks.
Each line describes one of the bookmarks in Emacs.
Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.
\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
  Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
\\[bookmark-bmenu-locate] -- display (in minibuffer) location of this bookmark.
\\[bookmark-bmenu-1-window] -- select this bookmark in full-frame window.
\\[bookmark-bmenu-2-window] -- select this bookmark in one window,
  together with bookmark selected before this one in another window.
\\[bookmark-bmenu-this-window] -- select this bookmark in place of the bookmark menu buffer.
\\[bookmark-bmenu-other-window] -- select this bookmark in another window,
  so the bookmark menu bookmark remains visible in its window.
\\[bookmark-bmenu-switch-other-window] -- switch the other window to this bookmark.
\\[bookmark-bmenu-rename] -- rename this bookmark \(prompts for new name\).
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file \(prompts for new file\).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
  With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
  With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
  in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
\\[bookmark-bmenu-edit-annotation] -- edit the annotation for the current bookmark."
  (kill-all-local-variables)
  (use-local-map bookmark-bmenu-mode-map)
  (setq truncate-lines t)
  (setq buffer-read-only t)
  (setq major-mode 'bookmark-bmenu-mode)
  (setq mode-name "Bookmark Menu")
  (run-mode-hooks 'bookmark-bmenu-mode-hook))


(defun bookmark-bmenu-toggle-filenames (&optional show)
  "Toggle whether filenames are shown in the bookmark list.
Optional argument SHOW means show them unconditionally."
  (interactive)
  (cond
   (show
    (setq bookmark-bmenu-toggle-filenames nil)
    (bookmark-bmenu-show-filenames)
    (setq bookmark-bmenu-toggle-filenames t))
   (bookmark-bmenu-toggle-filenames
    (bookmark-bmenu-hide-filenames)
    (setq bookmark-bmenu-toggle-filenames nil))
   (t
    (bookmark-bmenu-show-filenames)
    (setq bookmark-bmenu-toggle-filenames t))))


(defun bookmark-bmenu-show-filenames (&optional force)
  (if (and (not force) bookmark-bmenu-toggle-filenames)
      nil ;already shown, so do nothing
    (save-excursion
      (save-window-excursion
        (goto-char (point-min))
        (forward-line 2)
        (setq bookmark-bmenu-hidden-bookmarks ())
        (let ((inhibit-read-only t))
          (while (< (point) (point-max))
            (let ((bmrk (bookmark-bmenu-bookmark)))
              (setq bookmark-bmenu-hidden-bookmarks
                    (cons bmrk bookmark-bmenu-hidden-bookmarks))
	      (let ((start (save-excursion (end-of-line) (point))))
		(move-to-column bookmark-bmenu-file-column t)
		;; Strip off `mouse-face' from the white spaces region.
		(if (and (display-color-p) (display-mouse-p))
		    (remove-text-properties start (point)
					    '(mouse-face nil help-echo nil))))
	      (delete-region (point) (progn (end-of-line) (point)))
              (insert "  ")
              ;; Pass the NO-HISTORY arg:
              (bookmark-insert-location bmrk t)
              (forward-line 1))))))))


(defun bookmark-bmenu-hide-filenames (&optional force)
  (if (and (not force) bookmark-bmenu-toggle-filenames)
      ;; nothing to hide if above is nil
      (save-excursion
        (save-window-excursion
          (goto-char (point-min))
          (forward-line 2)
          (setq bookmark-bmenu-hidden-bookmarks
                (nreverse bookmark-bmenu-hidden-bookmarks))
          (save-excursion
            (goto-char (point-min))
            (search-forward "Bookmark")
            (backward-word 1)
            (setq bookmark-bmenu-bookmark-column (current-column)))
          (save-excursion
            (let ((inhibit-read-only t))
              (while bookmark-bmenu-hidden-bookmarks
                (move-to-column bookmark-bmenu-bookmark-column t)
                (bookmark-kill-line)
		(let ((start (point)))
		  (insert (car bookmark-bmenu-hidden-bookmarks))
		  (if (and (display-color-p) (display-mouse-p))
		      (add-text-properties
		       start
		       (save-excursion (re-search-backward
					"[^ \t]")
				       (1+ (point)))
		       '(mouse-face highlight
			 follow-link t
			 help-echo
			 "mouse-2: go to this bookmark in other window"))))
                (setq bookmark-bmenu-hidden-bookmarks
                      (cdr bookmark-bmenu-hidden-bookmarks))
                (forward-line 1))))))))


(defun bookmark-bmenu-check-position ()
  ;; Returns non-nil if on a line with a bookmark.
  ;; (The actual value returned is bookmark-alist).
  ;; Else reposition and try again, else return nil.
  (cond ((< (count-lines (point-min) (point)) 2)
         (goto-char (point-min))
         (forward-line 2)
         bookmark-alist)
        ((and (bolp) (eobp))
         (beginning-of-line 0)
         bookmark-alist)
        (t
         bookmark-alist)))


(defun bookmark-bmenu-bookmark ()
  ;; return a string which is bookmark of this line.
  (if (bookmark-bmenu-check-position)
      (save-excursion
        (save-window-excursion
          (goto-char (point-min))
          (search-forward "Bookmark")
          (backward-word 1)
          (setq bookmark-bmenu-bookmark-column (current-column)))))
  (if bookmark-bmenu-toggle-filenames
      (bookmark-bmenu-hide-filenames))
  (save-excursion
    (save-window-excursion
      (beginning-of-line)
      (forward-char bookmark-bmenu-bookmark-column)
      (prog1
          (buffer-substring-no-properties (point)
                            (progn
                              (end-of-line)
                              (point)))
        ;; well, this is certainly crystal-clear:
        (if bookmark-bmenu-toggle-filenames
            (bookmark-bmenu-toggle-filenames t))))))


(defun bookmark-show-annotation (bookmark)
  "Display the annotation for bookmark named BOOKMARK in a buffer,
if an annotation exists."
  (let ((annotation (bookmark-get-annotation bookmark)))
    (if (and annotation (not (string-equal annotation "")))
        (save-excursion
          (let ((old-buf (current-buffer)))
            (pop-to-buffer (get-buffer-create "*Bookmark Annotation*") t)
            (delete-region (point-min) (point-max))
            ;; (insert (concat "Annotation for bookmark '" bookmark "':\n\n"))
            (insert annotation)
            (goto-char (point-min))
            (pop-to-buffer old-buf))))))


(defun bookmark-show-all-annotations ()
  "Display the annotations for all bookmarks in a buffer."
  (let ((old-buf (current-buffer)))
    (pop-to-buffer (get-buffer-create "*Bookmark Annotation*") t)
    (delete-region (point-min) (point-max))
    (mapc
     (lambda (full-record)
       (let* ((name (bookmark-name-from-full-record full-record))
              (ann  (bookmark-get-annotation name)))
         (insert (concat name ":\n"))
         (if (and ann (not (string-equal ann "")))
             ;; insert the annotation, indented by 4 spaces.
             (progn
               (save-excursion (insert ann) (unless (bolp)
                                              (insert "\n")))
               (while (< (point) (point-max))
                 (beginning-of-line) ; paranoia
                 (insert "    ")
                 (forward-line)
                 (end-of-line))))))
     bookmark-alist)
    (goto-char (point-min))
    (pop-to-buffer old-buf)))


(defun bookmark-bmenu-mark ()
  "Mark bookmark on this line to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
  (interactive)
  (beginning-of-line)
  (if (bookmark-bmenu-check-position)
      (let ((inhibit-read-only t))
        (delete-char 1)
        (insert ?>)
        (forward-line 1)
        (bookmark-bmenu-check-position))))


(defun bookmark-bmenu-select ()
  "Select this line's bookmark; also display bookmarks marked with `>'.
You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] command."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (let ((bmrk (bookmark-bmenu-bookmark))
            (menu (current-buffer))
            (others ())
            tem)
        (goto-char (point-min))
        (while (re-search-forward "^>" nil t)
          (setq tem (bookmark-bmenu-bookmark))
          (let ((inhibit-read-only t))
            (delete-char -1)
            (insert ?\s))
          (or (string-equal tem bmrk)
              (member tem others)
              (setq others (cons tem others))))
        (setq others (nreverse others)
              tem (/ (1- (frame-height)) (1+ (length others))))
        (delete-other-windows)
        (bookmark-jump bmrk)
        (bury-buffer menu)
        (if others
            (while others
              (split-window nil tem)
              (other-window 1)
              (bookmark-jump (car others))
              (setq others (cdr others)))
          (other-window 1)))))


(defun bookmark-bmenu-save (parg)
  "Save the current list into a bookmark file.
With a prefix arg, prompts for a file to save them in."
  (interactive "P")
  (save-excursion
    (save-window-excursion
      (bookmark-save parg))))


(defun bookmark-bmenu-load ()
  "Load the bookmark file and rebuild the bookmark menu-buffer."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (save-excursion
        (save-window-excursion
          ;; This will call `bookmark-bmenu-list'
          (call-interactively 'bookmark-load)))))


(defun bookmark-bmenu-1-window ()
  "Select this line's bookmark, alone, in full frame."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (progn
        (bookmark-jump (bookmark-bmenu-bookmark))
        (bury-buffer (other-buffer))
        (delete-other-windows))))


(defun bookmark-bmenu-2-window ()
  "Select this line's bookmark, with previous buffer in second window."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (let ((bmrk (bookmark-bmenu-bookmark))
            (menu (current-buffer))
            (pop-up-windows t))
        (delete-other-windows)
        (switch-to-buffer (other-buffer))
        (let ((bookmark-automatically-show-annotations nil)) ;FIXME: needed?
          (bookmark--jump-via bmrk 'pop-to-buffer))
        (bury-buffer menu))))


(defun bookmark-bmenu-this-window ()
  "Select this line's bookmark in this window."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (bookmark-jump (bookmark-bmenu-bookmark))))


(defun bookmark-bmenu-other-window ()
  "Select this line's bookmark in other window, leaving bookmark menu visible."
  (interactive)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (if (bookmark-bmenu-check-position)
        (let ((bookmark-automatically-show-annotations t)) ;FIXME: needed?
          (bookmark--jump-via bookmark 'switch-to-buffer-other-window)))))


(defun bookmark-bmenu-switch-other-window ()
  "Make the other window select this line's bookmark.
The current window remains selected."
  (interactive)
  (let ((bookmark (bookmark-bmenu-bookmark))
        (pop-up-windows t)
        same-window-buffer-names
        same-window-regexps)
    (if (bookmark-bmenu-check-position)
        (let ((bookmark-automatically-show-annotations t)) ;FIXME: needed?
          (bookmark--jump-via bookmark 'display-buffer)))))

(defun bookmark-bmenu-other-window-with-mouse (event)
  "Select bookmark at the mouse pointer in other window, leaving bookmark menu visible."
  (interactive "e")
  (with-current-buffer (window-buffer (posn-window (event-end event)))
    (save-excursion
      (goto-char (posn-point (event-end event)))
      (bookmark-bmenu-other-window))))


(defun bookmark-bmenu-show-annotation ()
  "Show the annotation for the current bookmark in another window."
  (interactive)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (if (bookmark-bmenu-check-position)
	(bookmark-show-annotation bookmark))))


(defun bookmark-bmenu-show-all-annotations ()
  "Show the annotation for all bookmarks in another window."
  (interactive)
  (bookmark-show-all-annotations))


(defun bookmark-bmenu-edit-annotation ()
  "Edit the annotation for the current bookmark in another window."
  (interactive)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (if (bookmark-bmenu-check-position)
	(bookmark-edit-annotation bookmark))))


(defun bookmark-bmenu-unmark (&optional backup)
  "Cancel all requested operations on bookmark on this line and move down.
Optional BACKUP means move up."
  (interactive "P")
  (beginning-of-line)
  (if (bookmark-bmenu-check-position)
      (progn
        (let ((inhibit-read-only t))
          (delete-char 1)
          ;; any flags to reset according to circumstances?  How about a
          ;; flag indicating whether this bookmark is being visited?
          ;; well, we don't have this now, so maybe later.
          (insert " "))
        (forward-line (if backup -1 1))
        (bookmark-bmenu-check-position))))


(defun bookmark-bmenu-backup-unmark ()
  "Move up and cancel all requested operations on bookmark on line above."
  (interactive)
  (forward-line -1)
  (if (bookmark-bmenu-check-position)
      (progn
        (bookmark-bmenu-unmark)
        (forward-line -1)
        (bookmark-bmenu-check-position))))


(defun bookmark-bmenu-delete ()
  "Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive)
  (beginning-of-line)
  (if (bookmark-bmenu-check-position)
      (let ((inhibit-read-only t))
        (delete-char 1)
        (insert ?D)
        (forward-line 1)
        (bookmark-bmenu-check-position))))


(defun bookmark-bmenu-delete-backwards ()
  "Mark bookmark on this line to be deleted, then move up one line.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive)
  (bookmark-bmenu-delete)
  (forward-line -2)
  (if (bookmark-bmenu-check-position)
      (forward-line 1))
  (bookmark-bmenu-check-position))


(defun bookmark-bmenu-execute-deletions ()
  "Delete bookmarks marked with \\<Buffer-menu-mode-map>\\[Buffer-menu-delete] commands."
  (interactive)
  (message "Deleting bookmarks...")
  (let ((hide-em bookmark-bmenu-toggle-filenames)
        (o-point  (point))
        (o-str    (save-excursion
                    (beginning-of-line)
                    (if (looking-at "^D")
                        nil
                      (buffer-substring
                       (point)
                       (progn (end-of-line) (point))))))
        (o-col     (current-column)))
    (if hide-em (bookmark-bmenu-hide-filenames))
    (setq bookmark-bmenu-toggle-filenames nil)
    (goto-char (point-min))
    (forward-line 1)
    (while (re-search-forward "^D" (point-max) t)
      (bookmark-delete (bookmark-bmenu-bookmark) t)) ; pass BATCH arg
    (bookmark-bmenu-list)
    (setq bookmark-bmenu-toggle-filenames hide-em)
    (if bookmark-bmenu-toggle-filenames
        (bookmark-bmenu-toggle-filenames t))
    (if o-str
        (progn
          (goto-char (point-min))
          (search-forward o-str)
          (beginning-of-line)
          (forward-char o-col))
      (goto-char o-point))
    (beginning-of-line)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))
    (message "Deleting bookmarks...done")
    ))


(defun bookmark-bmenu-rename ()
  "Rename bookmark on current line.  Prompts for a new name."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (let ((bmrk (bookmark-bmenu-bookmark))
            (thispoint (point)))
        (bookmark-rename bmrk)
        (bookmark-bmenu-list)
        (goto-char thispoint))))


(defun bookmark-bmenu-locate ()
  "Display location of this bookmark.  Displays in the minibuffer."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (let ((bmrk (bookmark-bmenu-bookmark)))
        (message "%s" (bookmark-location bmrk)))))

(defun bookmark-bmenu-relocate ()
  "Change the file path of the bookmark on the current line,
  prompting with completion for the new path."
  (interactive)
  (if (bookmark-bmenu-check-position)
      (let ((bmrk (bookmark-bmenu-bookmark))
            (thispoint (point)))
        (bookmark-relocate bmrk)
        (goto-char thispoint))))

\f
;;; Menu bar stuff.  Prefix is "bookmark-menu".

(defun bookmark-menu-popup-paned-menu (event name entries)
  "Pop up multi-paned menu at EVENT, return string chosen from ENTRIES.
That is, ENTRIES is a list of strings which appear as the choices
in the menu.
The number of panes depends on the number of entries.
The visible entries are truncated to `bookmark-menu-length', but the
strings returned are not."
  (let ((f-height (/ (frame-height) 2))
	(pane-list nil)
	(iter 0))
    (while entries
      (let (lst
	    (count 0))
	(while (and (< count f-height) entries)
	  (let ((str (car entries)))
	    (push (cons
		   (if (> (length str) bookmark-menu-length)
		       (substring str 0 bookmark-menu-length)
		     str)
		   str)
		  lst)
	    (setq entries (cdr entries))
	    (setq count (1+ count))))
	(setq iter (1+ iter))
	(push (cons
	       (format "-*- %s (%d) -*-" name iter)
	       (nreverse lst))
	      pane-list)))

    ;; Popup the menu and return the string.
    (x-popup-menu event (cons (concat "-*- " name " -*-")
			      (nreverse pane-list)))))


;; Thanks to Roland McGrath for fixing menubar.el so that the
;; following works, and for explaining what to do to make it work.

;; We MUST autoload EACH form used to set up this variable's value, so
;; that the whole job is done in loaddefs.el.

;; Emacs menubar stuff.

;;;###autoload
(defvar menu-bar-bookmark-map
  (let ((map (make-sparse-keymap "Bookmark functions")))
    (define-key map [load]	'("Load a Bookmark File..." . bookmark-load))
    (define-key map [write]	'("Save Bookmarks As..." . bookmark-write))
    (define-key map [save]	'("Save Bookmarks" . bookmark-save))
    (define-key map [edit]	'("Edit Bookmark List" . bookmark-bmenu-list))
    (define-key map [delete]	'("Delete Bookmark..." . bookmark-delete))
    (define-key map [rename]	'("Rename Bookmark..." . bookmark-rename))
    (define-key map [locate]	'("Insert Location..." . bookmark-locate))
    (define-key map [insert]	'("Insert Contents..." . bookmark-insert))
    (define-key map [set]	'("Set Bookmark..." . bookmark-set))
    (define-key map [jump]	'("Jump to Bookmark..." . bookmark-jump))
    map))

;;;###autoload
(defalias 'menu-bar-bookmark-map menu-bar-bookmark-map)

;; make bookmarks appear toward the right side of the menu.
(if (boundp 'menu-bar-final-items)
    (if menu-bar-final-items
        (setq menu-bar-final-items
              (cons 'bookmark menu-bar-final-items)))
  (setq menu-bar-final-items '(bookmark)))

;;;; end bookmark menu stuff ;;;;

\f
;; Load Hook
(defvar bookmark-load-hook nil
  "Hook run at the end of loading bookmark.")

;; Exit Hook, called from kill-emacs-hook
(defvar bookmark-exit-hook nil
  "Hook run when Emacs exits.")

(define-obsolete-variable-alias 'bookmark-exit-hooks 'bookmark-exit-hook "22.1")

(defun bookmark-exit-hook-internal ()
  "Save bookmark state, if necessary, at Emacs exit time.
This also runs `bookmark-exit-hook'."
  (run-hooks 'bookmark-exit-hook)
  (and bookmark-alist
       (bookmark-time-to-save-p t)
       (bookmark-save)))

(add-hook 'kill-emacs-hook 'bookmark-exit-hook-internal)


(run-hooks 'bookmark-load-hook)

(provide 'bookmark)

;; arch-tag: 139f519a-dd0c-4b8d-8b5d-f9fcf53ca8f6
;;; bookmark.el ends here

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-16 20:45     ` Drew Adams
@ 2008-10-16 21:15       ` Eli Zaretskii
  2008-10-16 21:58         ` Drew Adams
  2008-10-17 12:38         ` Eli Zaretskii
  0 siblings, 2 replies; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-16 21:15 UTC (permalink / raw)
  To: Drew Adams; +Cc: 1183, bug-gnu-emacs, emacs-pretest-bug

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <emacs-pretest-bug@gnu.org>, <bug-gnu-emacs@gnu.org>
> Date: Thu, 16 Oct 2008 13:45:21 -0700
> 
> > If so, this is expected:
> 
> Well it certainly isn't expected in Emacs 20, 21, or 22, where ediff-buffers
> works perfectly for these two files. Why this change for Emacs 23? What's the
> gain?

Sorry, you are right: Emacs 22 also uses --binary, but doesn't expose
this problem.  So I guess something else is at work here.  I'll look
closer when I have time.

> And how to change the buffers (e.g. line endings) so ediff-buffers will compare
> them correctly?

"C-x RET f unix RET C-x C-s".






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-16 21:15       ` Eli Zaretskii
@ 2008-10-16 21:58         ` Drew Adams
  2008-10-17 12:38         ` Eli Zaretskii
  1 sibling, 0 replies; 27+ messages in thread
From: Drew Adams @ 2008-10-16 21:58 UTC (permalink / raw)
  To: 'Eli Zaretskii'; +Cc: 1183, bug-gnu-emacs, emacs-pretest-bug

> From: Eli Zaretskii Sent: Thursday, October 16, 2008 2:16 PM
> > From: "Drew Adams" Date: Thu, 16 Oct 2008 13:45:21 -0700
> > 
> > > If so, this is expected:
> > 
> > Well it certainly isn't expected in Emacs 20, 21, or 22, 
> > where ediff-buffers works perfectly for these two files.
> > Why this change for Emacs 23? What's the gain?
> 
> Sorry, you are right: Emacs 22 also uses --binary, but doesn't expose
> this problem.  So I guess something else is at work here.  I'll look
> closer when I have time.

Thanks, Eli. 

BTW, I don't know if it's related, but I just filed bug #1187, which has to do
with reading characters differently in Emacs 23 from Emacs 22. Maybe the two are
related in some way.

> > And how to change the buffers (e.g. line endings) so 
> > ediff-buffers will compare them correctly?
> 
> "C-x RET f unix RET C-x C-s".

But that would save the file with the new line endings. What if I don't want to
do that? ediff-buffers should not require you to save a file differently or even
to have a file associated with the buffer.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-16 21:15       ` Eli Zaretskii
  2008-10-16 21:58         ` Drew Adams
@ 2008-10-17 12:38         ` Eli Zaretskii
  2008-10-17 14:36           ` Drew Adams
  2008-10-17 16:02           ` Stefan Monnier
  1 sibling, 2 replies; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-17 12:38 UTC (permalink / raw)
  To: 1183; +Cc: bug-gnu-emacs, Michael Kifer

> Date: Thu, 16 Oct 2008 23:15:45 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 1183@emacsbugs.donarmstrong.com, bug-gnu-emacs@gnu.org,
> 	emacs-pretest-bug@gnu.org
> 
> > From: "Drew Adams" <drew.adams@oracle.com>
> > Cc: <emacs-pretest-bug@gnu.org>, <bug-gnu-emacs@gnu.org>
> > Date: Thu, 16 Oct 2008 13:45:21 -0700
> > 
> > > If so, this is expected:
> > 
> > Well it certainly isn't expected in Emacs 20, 21, or 22, where ediff-buffers
> > works perfectly for these two files. Why this change for Emacs 23? What's the
> > gain?
> 
> Sorry, you are right: Emacs 22 also uses --binary, but doesn't expose
> this problem.  So I guess something else is at work here.  I'll look
> closer when I have time.

Found the culprit.  It's in ediff-make-temp-file:

  (let* ( ...
  	 (coding-system-for-write
	  (ediff-with-current-buffer buff
	    (if (boundp 'buffer-file-coding-system)
		buffer-file-coding-system
	      ediff-coding-system-for-write)))

This let-binds the coding-system with which we will write the two
buffers to temporary files, to the original encoding of the files read
into the buffers being compared.  The temporary files are then
submitted to Diff, and that makes each line compare different because
of the different EOL format.

Emacs 22 unconditionally uses ediff-coding-system-for-write here,
which is set to `no-conversion', so this problem does not happen in
Emacs 22.

This change was made in Aug 2007, and Michael Kifer wrote a bit later
on emacs-devel that using no-conversion was screwing some other
use-case (I couldn't find the description of that use-case, although
Michael said it was reported earlier on emacs-devel).

I could easily fix this particular problem by forcing the EOL
conversion to -unix, but I think there's a broader issue here, and we
might as well handle it now.  The issue is this: suppose we have two
buffers whose text is identical in the internal Emacs representation
of characters, but whose values of buffer-file-coding-system differ--
do we want such buffers to compare equal or not?  For example, the
buffers could contain Latin-2 text, with one buffer coming from a file
encoded in ISO-8859-2, the other coming from a UTF-8 file.

The current Ediff code will compare these buffers different.  If we
want them to compare equal instead, this means that `ediff' and
`ediff-buffers' will produce different results for the same two files.

The different EOL format is just a special case of this general
problem.

In a discussion in Oct 2007, Stefan said that using the buffer's
encoding is wrong, see:

  http://lists.gnu.org/archive/html/emacs-devel/2007-10/msg01381.html

Stefan wanted to use the equivalent of emacs-mule for Emacs 23 instead
of buffer's encoding, but do we have such an encoding now?  Is
no-conversion-multibyte it?  Or maybe utf-8 is good enough?

But first, we should decide whether we want such buffers to compare
equal or not.

We could also let them compare equal, but display a message to the
effect that the buffers define different encoding for saving them to
files.

Opinions?







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 12:38         ` Eli Zaretskii
@ 2008-10-17 14:36           ` Drew Adams
  2008-10-17 16:02           ` Stefan Monnier
  1 sibling, 0 replies; 27+ messages in thread
From: Drew Adams @ 2008-10-17 14:36 UTC (permalink / raw)
  To: 'Eli Zaretskii', 1183; +Cc: bug-gnu-emacs, 'Michael Kifer'

> From: Eli Zaretskii Sent: Friday, October 17, 2008 5:39 AM
> > > > If so, this is expected:
> > > 
> > > Well it certainly isn't expected in Emacs 20, 21, or 22, 
> > > where ediff-buffers works perfectly for these two files.
> > > Why this change for Emacs 23? What's the gain?
> > 
> > Sorry, you are right: Emacs 22 also uses --binary, but 
> > doesn't expose this problem.  So I guess something else is
> > at work here.  I'll look closer when I have time.
> 
> Found the culprit.  It's in ediff-make-temp-file:
> 
>   (let* ( ...
>   	 (coding-system-for-write
> 	  (ediff-with-current-buffer buff
> 	    (if (boundp 'buffer-file-coding-system)
> 		buffer-file-coding-system
> 	      ediff-coding-system-for-write)))
> 
> This let-binds the coding-system with which we will write the two
> buffers to temporary files, to the original encoding of the files read
> into the buffers being compared.  The temporary files are then
> submitted to Diff, and that makes each line compare different because
> of the different EOL format.
> 
> Emacs 22 unconditionally uses ediff-coding-system-for-write here,
> which is set to `no-conversion', so this problem does not happen in
> Emacs 22.
> 
> This change was made in Aug 2007, and Michael Kifer wrote a bit later
> on emacs-devel that using no-conversion was screwing some other
> use-case (I couldn't find the description of that use-case, although
> Michael said it was reported earlier on emacs-devel).
> 
> I could easily fix this particular problem by forcing the EOL
> conversion to -unix, but I think there's a broader issue here, and we
> might as well handle it now.  The issue is this: suppose we have two
> buffers whose text is identical in the internal Emacs representation
> of characters, but whose values of buffer-file-coding-system differ--
> do we want such buffers to compare equal or not?  For example, the
> buffers could contain Latin-2 text, with one buffer coming from a file
> encoded in ISO-8859-2, the other coming from a UTF-8 file.
> 
> The current Ediff code will compare these buffers different.  If we
> want them to compare equal instead, this means that `ediff' and
> `ediff-buffers' will produce different results for the same two files.
> 
> The different EOL format is just a special case of this general
> problem.
> 
> In a discussion in Oct 2007, Stefan said that using the buffer's
> encoding is wrong, see:
> 
>   http://lists.gnu.org/archive/html/emacs-devel/2007-10/msg01381.html
> 
> Stefan wanted to use the equivalent of emacs-mule for Emacs 23 instead
> of buffer's encoding, but do we have such an encoding now?  Is
> no-conversion-multibyte it?  Or maybe utf-8 is good enough?
> 
> But first, we should decide whether we want such buffers to compare
> equal or not.
> 
> We could also let them compare equal, but display a message to the
> effect that the buffers define different encoding for saving them to
> files.
> 
> Opinions?

I don't have much of an opinion because of ignorance in this area. If you are
asking whether ediff should treat buffers as different when they differ only by
encoding or line endings, I'd say probably yes and know: It would be good to
give the user all knowledge that ediff has, and then let the user control how to
compare. A message could let the user know about line-ending differences, for
example, and ask if you want to compare using only Emacs's internal encoding (or
whatever the correct terminology is) - that is, ignoring just the line-ending
difference.

IOW, give the user the knowledge and control. What's unacceptable is for Emacs
to throw up its hands and just say the two are different, without saying/showing
how they differ. The user should be able to drill down and see all possible
differences, perhaps showing different kinds of differences in different ways.

I'll leave the rest to you guys who know about this stuff. You get the drift of
what's missing/wrong. Thx.








^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 12:38         ` Eli Zaretskii
  2008-10-17 14:36           ` Drew Adams
@ 2008-10-17 16:02           ` Stefan Monnier
  2008-10-17 16:48             ` Drew Adams
  2008-10-17 18:34             ` Eli Zaretskii
  1 sibling, 2 replies; 27+ messages in thread
From: Stefan Monnier @ 2008-10-17 16:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 1183, bug-gnu-emacs, Michael Kifer

> In a discussion in Oct 2007, Stefan said that using the buffer's
> encoding is wrong, see:

>   http://lists.gnu.org/archive/html/emacs-devel/2007-10/msg01381.html

> Stefan wanted to use the equivalent of emacs-mule for Emacs 23 instead
> of buffer's encoding, but do we have such an encoding now?  Is
> no-conversion-multibyte it?  Or maybe utf-8 is good enough?

As mentioned in the past, I think `no-conversion' should be killed
because it's confusing.  As for the problem at hand, utf-8-emacs-unix is
what we want to use.

> But first, we should decide whether we want such buffers to compare
> equal or not.

I believe we do, because it's called ediff-buffers.  There's ediff-files
for when you want to compare the files.

> We could also let them compare equal, but display a message to the
> effect that the buffers define different encoding for saving them to
> files.
> Opinions?

That would be fine, indeed.


        Stefan







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 16:02           ` Stefan Monnier
@ 2008-10-17 16:48             ` Drew Adams
  2008-10-17 17:05               ` Michael Kifer
  2008-10-17 18:19               ` Eli Zaretskii
  2008-10-17 18:34             ` Eli Zaretskii
  1 sibling, 2 replies; 27+ messages in thread
From: Drew Adams @ 2008-10-17 16:48 UTC (permalink / raw)
  To: 'Stefan Monnier', 1183, 'Eli Zaretskii'
  Cc: bug-gnu-emacs, 'Michael Kifer'

> > But first, we should decide whether we want such buffers to compare
> > equal or not.
> 
> I believe we do, because it's called ediff-buffers.  There's 
> ediff-files for when you want to compare the files.

That's terrible. Ediff-buffers has always been usable directly for buffers
visiting files also. 

It's OK for ediff-buffers to be more refined than before, to be able to take
into account current encodings etc. for the buffers, but it should inform the
user of the situation and let the user, if s?he wants, proceed to compare the
buffers using the same encodings etc. - or whatever is necessary to see the
actual textual differences, beyond encoding etc. differences.

The same behavior as previously (Emacs 22) should be available as a user choice
if the only differences are line endings, encodings, etc. And such differences
as line endings should at least be treated as differences and shown as such.
It's no good to just say the buffers are different, without offering more info
than that.

IOW, ediff-buffers should be at least as useful as it was before. Adding coding
diffs should be a plus, not a minus. Simply punting, showing a single giant diff
with no possible refinement and no explanation, is not helpful.

> > We could also let them compare equal, but display a message to the
> > effect that the buffers define different encoding for saving them to
> > files. Opinions?
> 
> That would be fine, indeed.

Fine, but not enough. If a user wants to see the textual differences between the
two buffers, the info that the encodings are different is not helpful enough to
get the job done. In the case described, there are real textual differences (an
added Lisp sexp), and ediff-buffers is not at all helpful in showing them.








^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 16:48             ` Drew Adams
@ 2008-10-17 17:05               ` Michael Kifer
  2008-10-17 17:17                 ` Drew Adams
  2008-10-17 18:19               ` Eli Zaretskii
  1 sibling, 1 reply; 27+ messages in thread
From: Michael Kifer @ 2008-10-17 17:05 UTC (permalink / raw)
  To: Drew Adams; +Cc: 1183, 'Michael Kifer', bug-gnu-emacs



On Fri, 17 Oct 2008 09:48:06 -0700
"Drew Adams" <drew.adams@oracle.com> wrote:

> > > But first, we should decide whether we want such buffers to compare
> > > equal or not.
> > 
> > I believe we do, because it's called ediff-buffers.  There's 
> > ediff-files for when you want to compare the files.
> 
> That's terrible. Ediff-buffers has always been usable directly for buffers
> visiting files also. 

I didn't see the original post, but the general idea was that whenever things
look the same in Emacs they should be treated as equal (or equal module spaces).
I do not think the user should be bothered with encodings. Copying from buffer
to buffer should also be transparent. (And ediff-files and ediff-buffers should
produce the same results.)

Unfortunately, I have not been following the developments in the last few
years, and my knowledge of the mechanics became rusty.


	--michael  


> It's OK for ediff-buffers to be more refined than before, to be able to take
> into account current encodings etc. for the buffers, but it should inform the
> user of the situation and let the user, if s?he wants, proceed to compare the
> buffers using the same encodings etc. - or whatever is necessary to see the
> actual textual differences, beyond encoding etc. differences.
> 
> The same behavior as previously (Emacs 22) should be available as a user choice
> if the only differences are line endings, encodings, etc. And such differences
> as line endings should at least be treated as differences and shown as such.
> It's no good to just say the buffers are different, without offering more info
> than that.
> 
> IOW, ediff-buffers should be at least as useful as it was before. Adding coding
> diffs should be a plus, not a minus. Simply punting, showing a single giant diff
> with no possible refinement and no explanation, is not helpful.
> 
> > > We could also let them compare equal, but display a message to the
> > > effect that the buffers define different encoding for saving them to
> > > files. Opinions?
> > 
> > That would be fine, indeed.
> 
> Fine, but not enough. If a user wants to see the textual differences between the
> two buffers, the info that the encodings are different is not helpful enough to
> get the job done. In the case described, there are real textual differences (an
> added Lisp sexp), and ediff-buffers is not at all helpful in showing them.
> 
> 
> 






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 17:05               ` Michael Kifer
@ 2008-10-17 17:17                 ` Drew Adams
  2008-10-17 18:15                   ` Eli Zaretskii
  0 siblings, 1 reply; 27+ messages in thread
From: Drew Adams @ 2008-10-17 17:17 UTC (permalink / raw)
  To: kifer; +Cc: 1183, 'Michael Kifer', bug-gnu-emacs

> > > > But first, we should decide whether we want such 
> > > > buffers to compare equal or not.
> > > 
> > > I believe we do, because it's called ediff-buffers.  There's 
> > > ediff-files for when you want to compare the files.
> > 
> > That's terrible. Ediff-buffers has always been usable 
> > directly for buffers visiting files also. 
> 
> I didn't see the original post, but the general idea was that 
> whenever things look the same in Emacs they should be treated
> as equal (or equal module spaces). I do not think the user
> should be bothered with encodings. Copying from buffer
> to buffer should also be transparent. (And ediff-files and 
> ediff-buffers should produce the same results.)
> 
> Unfortunately, I have not been following the developments in 
> the last few years, and my knowledge of the mechanics became rusty.

Everything Michael said sounds right to me.

IMO, it would be OK for ediff-buffers, perhaps optionally, to report encoding
differences *also*, but that's not what ediff-buffers is really about.

It's about detecting textual differences: `foo' vs `fog'. If it cannot tell you
that `foo' is different from `fog', and it only tells you that the two buffers
are (somehow) different, then it is being less helpful than before, not more.








^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 17:17                 ` Drew Adams
@ 2008-10-17 18:15                   ` Eli Zaretskii
  2008-10-17 18:35                     ` Drew Adams
  0 siblings, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-17 18:15 UTC (permalink / raw)
  To: Drew Adams; +Cc: 1183, bug-gnu-emacs, kifer, kifer

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: "'Stefan Monnier'" <monnier@iro.umontreal.ca>,
>         <1183@emacsbugs.donarmstrong.com>, "'Eli Zaretskii'" <eliz@gnu.org>,
>         <bug-gnu-emacs@gnu.org>, "'Michael Kifer'" <kifer@cs.stonybrook.edu>
> Date: Fri, 17 Oct 2008 10:17:27 -0700
> 
> > > > > But first, we should decide whether we want such 
> > > > > buffers to compare equal or not.
> > > > 
> > > > I believe we do, because it's called ediff-buffers.  There's 
> > > > ediff-files for when you want to compare the files.
> > > 
> > > That's terrible. Ediff-buffers has always been usable 
> > > directly for buffers visiting files also. 
> > 
> > I didn't see the original post, but the general idea was that 
> > whenever things look the same in Emacs they should be treated
> > as equal (or equal module spaces). I do not think the user
> > should be bothered with encodings. Copying from buffer
> > to buffer should also be transparent. (And ediff-files and 
> > ediff-buffers should produce the same results.)
> > 
> > Unfortunately, I have not been following the developments in 
> > the last few years, and my knowledge of the mechanics became rusty.
> 
> Everything Michael said sounds right to me.

Then why did you say "that's terrible" in response to Stefan who
expressed the same view as Michael?  They both say that what is
_displayed_ the same in Emacs should compare equal in ediff-buffers.

OTOH, "M-x ediff" that compares _files_ will still show differences
for identical text encoded differently in each of the files.






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 16:48             ` Drew Adams
  2008-10-17 17:05               ` Michael Kifer
@ 2008-10-17 18:19               ` Eli Zaretskii
  2008-10-17 18:35                 ` Drew Adams
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-17 18:19 UTC (permalink / raw)
  To: Drew Adams; +Cc: 1183, bug-gnu-emacs, kifer

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <bug-gnu-emacs@gnu.org>, "'Michael Kifer'" <kifer@cs.stonybrook.edu>
> Date: Fri, 17 Oct 2008 09:48:06 -0700
> 
> > > We could also let them compare equal, but display a message to the
> > > effect that the buffers define different encoding for saving them to
> > > files. Opinions?
> > 
> > That would be fine, indeed.
> 
> Fine, but not enough. If a user wants to see the textual differences between the
> two buffers, the info that the encodings are different is not helpful enough to
> get the job done.

You misunderstood.  My suggestion, to which Stefan agreed, was that
_in_addition_ to showing any textual differences, ediff-buffers would
display a warning saying that the buffers _also_ differ in the way
they will be encoded on their disk files.

In the extreme case that the text is identical, only the warning will
remain as the single sign of a difference.

> In the case described, there are real textual differences (an
> added Lisp sexp), and ediff-buffers is not at all helpful in showing them.

After Ediff is fixed, you will see the diffs you see in Emacs 22, plus
a message saying that the EOL format is different.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 16:02           ` Stefan Monnier
  2008-10-17 16:48             ` Drew Adams
@ 2008-10-17 18:34             ` Eli Zaretskii
  2008-10-19  2:21               ` Stefan Monnier
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-17 18:34 UTC (permalink / raw)
  To: Stefan Monnier, Kenichi Handa; +Cc: 1183, bug-gnu-emacs, kifer

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 1183@emacsbugs.donarmstrong.com,  bug-gnu-emacs@gnu.org,  Michael Kifer <kifer@cs.stonybrook.edu>
> Date: Fri, 17 Oct 2008 12:02:11 -0400
> 
> > In a discussion in Oct 2007, Stefan said that using the buffer's
> > encoding is wrong, see:
> 
> >   http://lists.gnu.org/archive/html/emacs-devel/2007-10/msg01381.html
> 
> > Stefan wanted to use the equivalent of emacs-mule for Emacs 23 instead
> > of buffer's encoding, but do we have such an encoding now?  Is
> > no-conversion-multibyte it?  Or maybe utf-8 is good enough?
> 
> As mentioned in the past, I think `no-conversion' should be killed
> because it's confusing.  As for the problem at hand, utf-8-emacs-unix is
> what we want to use.

Suppose we write the temp files with utf-8-emacs-unix encoding--won't
that bite us when the output from Diff is then read with raw-text (see
ediff-exec-process)?

If raw-text won't work for utf-8-emacs output from Diff, is there a
better way than overriding the coding-system priorities with
`with-coding-priority'?

> > But first, we should decide whether we want such buffers to compare
> > equal or not.
> 
> I believe we do, because it's called ediff-buffers.  There's ediff-files
> for when you want to compare the files.

Don't you think having direct file comparison yield results that are
different from comparing buffers that visit those same files will be
confusing?






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 18:19               ` Eli Zaretskii
@ 2008-10-17 18:35                 ` Drew Adams
  0 siblings, 0 replies; 27+ messages in thread
From: Drew Adams @ 2008-10-17 18:35 UTC (permalink / raw)
  To: 'Eli Zaretskii'; +Cc: 1183, bug-gnu-emacs, kifer

> > > > We could also let them compare equal, but display a 
> > > > message to the effect that the buffers define different
> > > > encoding for saving them to files. Opinions?
> > > 
> > > That would be fine, indeed.
> > 
> > Fine, but not enough. If a user wants to see the textual 
> > differences between the two buffers, the info that the
> > encodings are different is not helpful enough to get the
> > job done.
> 
> You misunderstood.  My suggestion, to which Stefan agreed, was that
> _in_addition_ to showing any textual differences, ediff-buffers would
> display a warning saying that the buffers _also_ differ in the way
> they will be encoded on their disk files.
> 
> In the extreme case that the text is identical, only the warning will
> remain as the single sign of a difference.
> 
> > In the case described, there are real textual differences (an
> > added Lisp sexp), and ediff-buffers is not at all helpful 
> > in showing them.
> 
> After Ediff is fixed, you will see the diffs you see in Emacs 22, plus
> a message saying that the EOL format is different.

Sounds good. Sorry for misunderstanding. "Let them compare equal" threw me off;
I thought you were saying that the two buffers I reported on *should* be
considered equal by ediff-buffers.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 18:15                   ` Eli Zaretskii
@ 2008-10-17 18:35                     ` Drew Adams
  2008-10-18  3:17                       ` Michael Kifer
  0 siblings, 1 reply; 27+ messages in thread
From: Drew Adams @ 2008-10-17 18:35 UTC (permalink / raw)
  To: 'Eli Zaretskii'; +Cc: 1183, bug-gnu-emacs, kifer, kifer

> > > > > > But first, we should decide whether we want such 
> > > > > > buffers to compare equal or not.
> > > > > 
> > > > > I believe we do, because it's called ediff-buffers.  There's 
> > > > > ediff-files for when you want to compare the files.
> > > > 
> > > > That's terrible. Ediff-buffers has always been usable 
> > > > directly for buffers visiting files also. 
> > > 
> > > I didn't see the original post, but the general idea was that 
> > > whenever things look the same in Emacs they should be treated
> > > as equal (or equal module spaces). I do not think the user
> > > should be bothered with encodings. Copying from buffer
> > > to buffer should also be transparent. (And ediff-files and 
> > > ediff-buffers should produce the same results.)
> > > 
> > > Unfortunately, I have not been following the developments in 
> > > the last few years, and my knowledge of the mechanics 
> became rusty.
> > 
> > Everything Michael said sounds right to me.
> 
> Then why did you say "that's terrible" in response to Stefan who
> expressed the same view as Michael?  They both say that what is
> _displayed_ the same in Emacs should compare equal in ediff-buffers.

I guess I misunderstood. I thought that Stefan was saying that ediff-buffers
should continue to do what it is doing now: just show one big difference, with
no distinction of textual differences, and force users to use ediff-files to see
the textual differences. If he meant the same thing as Michael, then we agree.

The two buffers I reported on should *not* compare equal, but neither should
ediff-buffers just throw up its hands and say only "they're different somehow".
I mistakenly thought that Stefan was saying that ediff-buffers should not
distinguish their textual differences but should just report that they are
different (one big diff). IOW, I thought he was saying that the current behavior
is correct for ediff-buffers but that ediff-files should, as always, show the
textual differences.

> OTOH, "M-x ediff" that compares _files_ will still show differences
> for identical text encoded differently in each of the files.

Again, I have no problem with ediff commands also showing or otherwise
identifying encoding differences. 

What I objected to was that textual differences were being effectively lost,
because ediff-buffers just displays one big diff with identical, full-buffer
highlighting - it doesn't show the textual differences at all.

Sorry for any misunderstanding. It sounds now like we're all in more or less
agreement.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 18:35                     ` Drew Adams
@ 2008-10-18  3:17                       ` Michael Kifer
  2008-10-18  3:43                         ` Drew Adams
  2008-10-18  9:07                         ` Eli Zaretskii
  0 siblings, 2 replies; 27+ messages in thread
From: Michael Kifer @ 2008-10-18  3:17 UTC (permalink / raw)
  To: Drew Adams; +Cc: 1183, kifer, bug-gnu-emacs



On Fri, 17 Oct 2008 11:35:46 -0700
"Drew Adams" <drew.adams@oracle.com> wrote:

> > > > > > > But first, we should decide whether we want such 
> > > > > > > buffers to compare equal or not.
> > > > > > 
> > > > > > I believe we do, because it's called ediff-buffers.  There's 
> > > > > > ediff-files for when you want to compare the files.
> > > > > 
> > > > > That's terrible. Ediff-buffers has always been usable 
> > > > > directly for buffers visiting files also. 
> > > > 
> > > > I didn't see the original post, but the general idea was that 
> > > > whenever things look the same in Emacs they should be treated
> > > > as equal (or equal module spaces). I do not think the user
> > > > should be bothered with encodings. Copying from buffer
> > > > to buffer should also be transparent. (And ediff-files and 
> > > > ediff-buffers should produce the same results.)
> > > > 
> > > > Unfortunately, I have not been following the developments in 
> > > > the last few years, and my knowledge of the mechanics 
> > became rusty.
> > > 
> > > Everything Michael said sounds right to me.
> > 
> > Then why did you say "that's terrible" in response to Stefan who
> > expressed the same view as Michael?  They both say that what is
> > _displayed_ the same in Emacs should compare equal in ediff-buffers.
> 
> I guess I misunderstood. I thought that Stefan was saying that ediff-buffers
> should continue to do what it is doing now: just show one big difference, with
> no distinction of textual differences, and force users to use ediff-files to see
> the textual differences. If he meant the same thing as Michael, then we agree.
> 
> The two buffers I reported on should *not* compare equal, but neither should
> ediff-buffers just throw up its hands and say only "they're different somehow".

To make it clear, nobody was saying that the two buffers should be compared
equal. The issue is: how do we make diff (NOT ediff) to recognize that these
buffers are nearly identical modulo some minor stuff?

Ediff-buffers does almost the right thing (at least, was doing until things
changed in emacs). It would save the buffers in temp files using the *same*
encoding, so all that crap is pushed out of the way. Then it would run diff on
the temp files.  Since the encodings are the same, diff would find what is
different and then ediff will display that. (With all its complexity, ediff is
just a front-end for diff.) So, for ediff-buffers, the question is which
encoding to use.

For ediff-files things seem to be worse: it runs diff on the original files, so
if one has DOS line endings and the other does not then it all depends on what
diff does.  This is why sometimes you run ediff files on 2 files that are
nearly identical and get one big diff region equal to the entire file.
This is a bit annoying, but not too bad, since hitting * should fix the
problem: it would save the diff regions using the same encoding and will run
diff over them.

So, basically, it boils down to choosing the right encoding.
I am not sure which is right.
Long time ago, it was decided to use no-conversion. Then someone found a bad
case and suggested to use the buffer coding system *if* it is set.
This seemed to work better, but still had some problems.

Back then Stefan suggested emacs-mule instead of no-conversion, but for some
reason this was not done--don't remember why. He also said that things will
change in emacs 23, but I did not follow that development.

What has changed in emacs 23 with respect to this issue?

> I mistakenly thought that Stefan was saying that ediff-buffers should not
> distinguish their textual differences but should just report that they are
> different (one big diff). IOW, I thought he was saying that the current
> behavior
> is correct for ediff-buffers but that ediff-files should, as always, show the
> textual differences.
> 
> > OTOH, "M-x ediff" that compares _files_ will still show differences
> > for identical text encoded differently in each of the files.
> 
> Again, I have no problem with ediff commands also showing or otherwise
> identifying encoding differences. 

See above. The point was not that textual diffs should be lost, but that it
should be made possible for the diff program to recognize identical regions
(modulo dos line endings, etc) as such.


> What I objected to was that textual differences were being effectively lost,
> because ediff-buffers just displays one big diff with identical, full-buffer
> highlighting - it doesn't show the textual differences at all.


This was a misunderstanding.






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-18  3:17                       ` Michael Kifer
@ 2008-10-18  3:43                         ` Drew Adams
  2008-10-18  9:07                         ` Eli Zaretskii
  1 sibling, 0 replies; 27+ messages in thread
From: Drew Adams @ 2008-10-18  3:43 UTC (permalink / raw)
  To: kifer; +Cc: 1183, kifer, bug-gnu-emacs

> To make it clear, nobody was saying that the two buffers 
> should be compared
> equal. The issue is: how do we make diff (NOT ediff) to 
> recognize that these
> buffers are nearly identical modulo some minor stuff?
> 
> Ediff-buffers does almost the right thing (at least, was 
> doing until things
> changed in emacs). It would save the buffers in temp files 
> using the *same*
> encoding, so all that crap is pushed out of the way. Then it 
> would run diff on
> the temp files.  Since the encodings are the same, diff would 
> find what is
> different and then ediff will display that. (With all its 
> complexity, ediff is
> just a front-end for diff.) So, for ediff-buffers, the 
> question is which
> encoding to use.
> 
> For ediff-files things seem to be worse: it runs diff on the 
> original files, so
> if one has DOS line endings and the other does not then it 
> all depends on what
> diff does.  This is why sometimes you run ediff files on 2 
> files that are
> nearly identical and get one big diff region equal to the entire file.
> This is a bit annoying, but not too bad, since hitting * 
> should fix the
> problem: it would save the diff regions using the same 
> encoding and will run
> diff over them.
> 
> So, basically, it boils down to choosing the right encoding.
> I am not sure which is right.
> Long time ago, it was decided to use no-conversion. Then 
> someone found a bad
> case and suggested to use the buffer coding system *if* it is set.
> This seemed to work better, but still had some problems.
> 
> Back then Stefan suggested emacs-mule instead of 
> no-conversion, but for some
> reason this was not done--don't remember why. He also said 
> that things will
> change in emacs 23, but I did not follow that development.
> 
> What has changed in emacs 23 with respect to this issue?
> 
> > Again, I have no problem with ediff commands also showing 
> > or otherwise identifying encoding differences. 
> 
> See above. The point was not that textual diffs should be 
> lost, but that it should be made possible for the diff program
> to recognize identical regions
> (modulo dos line endings, etc) as such.

Got it. Thanks for a clear description. Sorry for having misunderstood.

FWIW, the need to hit `*' to refine for ediff in the situation you describe has
been true for a while, and it is no big deal. Generally, highlighting just the
differences is easier to read, but highlighting everything and then refining is
OK too, if need be.

Thanks to all for working on this.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-18  3:17                       ` Michael Kifer
  2008-10-18  3:43                         ` Drew Adams
@ 2008-10-18  9:07                         ` Eli Zaretskii
  2008-10-19  2:17                           ` Stefan Monnier
  1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-18  9:07 UTC (permalink / raw)
  To: kifer, handa; +Cc: 1183, bug-gnu-emacs, kifer

> Date: Fri, 17 Oct 2008 23:17:31 -0400
> From: Michael Kifer <kifer@cs.sunysb.edu>
> Cc: "'Eli Zaretskii'" <eliz@gnu.org>, <monnier@iro.umontreal.ca>,
>         <1183@emacsbugs.donarmstrong.com>, <bug-gnu-emacs@gnu.org>,
>         <kifer@cs.stonybrook.edu>
> 
> Ediff-buffers does almost the right thing (at least, was doing until things
> changed in emacs). It would save the buffers in temp files using the *same*
> encoding, so all that crap is pushed out of the way. Then it would run diff on
> the temp files.  Since the encodings are the same, diff would find what is
> different and then ediff will display that. (With all its complexity, ediff is
> just a front-end for diff.) So, for ediff-buffers, the question is which
> encoding to use.

The right encoding in Emacs 23 is utf-8-emacs-unix.  The problem with
that is that ediff-exec-process then uses raw-text to read the output
from Diff back into Emacs.  While raw-text is probably OK for reading
Diff output from comparing _files_, I'm afraid it will not be TRT for
reading output from comparing 2 temporary files encoded in
utf-8-emacs-unix.  If my fears are justified, I guess we will have to
modify ediff-exec-process so as to use utf-8-emacs-unix when
ediff-job-name has "buffers" in it.

> For ediff-files things seem to be worse: it runs diff on the original files, so
> if one has DOS line endings and the other does not then it all depends on what
> diff does.  This is why sometimes you run ediff files on 2 files that are
> nearly identical and get one big diff region equal to the entire file.
> This is a bit annoying, but not too bad, since hitting * should fix the
> problem: it would save the diff regions using the same encoding and will run
> diff over them.

Yes, but will hitting "*" help for Drew's use-case?  AFAIU, it will
"magically" show only the textual diffs, with no real explanation how
come the original display shows that every line is different, is that
right?

(Btw, "M-x ediff" actually does not pass the --binary switch to Diff,
so the original Ediff display is actually what Drew wanted, but let's
say we are doing the same on Unix, where changes in the EOL format are
reported by Diff as differences by default.)

> Back then Stefan suggested emacs-mule instead of no-conversion, but for some
> reason this was not done--don't remember why.

No special reason.

> He also said that things will change in emacs 23, but I did not
> follow that development.

See above.  I see that Stefan introduced emacs-internal into the Emacs
22 branch, but I don't see it on the trunk yet.  If and when it
arrives, we should use that one instead.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-18  9:07                         ` Eli Zaretskii
@ 2008-10-19  2:17                           ` Stefan Monnier
  2008-10-19  7:17                             ` Eli Zaretskii
                                               ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Stefan Monnier @ 2008-10-19  2:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 1183, kifer, handa, bug-gnu-emacs, kifer

>> Ediff-buffers does almost the right thing (at least, was doing until
>> things changed in emacs). It would save the buffers in temp files
>> using the *same* encoding, so all that crap is pushed out of the
>> way. Then it would run diff on the temp files.  Since the encodings
>> are the same, diff would find what is different and then ediff will
>> display that. (With all its complexity, ediff is just a front-end for
>> diff.) So, for ediff-buffers, the question is which encoding to use.

> The right encoding in Emacs 23 is utf-8-emacs-unix.

Indeed.  This encoding is now also available under the new name
`emacs-internal'. [ As you may have seen, I also added it to the
Emacs-22 branch, although I doubt we'll release anything from that
branch. ]

> The problem with that is that ediff-exec-process then uses raw-text to
> read the output from Diff back into Emacs.

Yes, raw-text is wrong.  It should probably use undecided at least, or
otherwise try to be more clever and use the encoding used by the
source files.

> While raw-text is probably OK for reading Diff output from comparing
> _files_,

There's worse, but for Unicode files (or for latin-1 files, or for ...)
it's kind of ugly.  I suspect that for utf-16 and Chinese it's
even worse.

> I'm afraid it will not be TRT for reading output from
> comparing 2 temporary files encoded in utf-8-emacs-unix.  If my fears
> are justified, I guess we will have to modify ediff-exec-process so as
> to use utf-8-emacs-unix when ediff-job-name has "buffers" in it.

Yes, that's part of the problem.

>> Back then Stefan suggested emacs-mule instead of no-conversion, but for some
>> reason this was not done--don't remember why.

> No special reason.

Actually, after looking at it a little bit, there are some reasons: the
encoding is currently set in ediff-make-temp-file which is used in many
different circumstances, some of which should use emacs-internal, but
maybe not all.  So there's more work to do in ediff to handle
encoding issues reliably.


        Stefan







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-17 18:34             ` Eli Zaretskii
@ 2008-10-19  2:21               ` Stefan Monnier
  0 siblings, 0 replies; 27+ messages in thread
From: Stefan Monnier @ 2008-10-19  2:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 1183, bug-gnu-emacs, kifer, Kenichi Handa

> Suppose we write the temp files with utf-8-emacs-unix encoding--won't
> that bite us when the output from Diff is then read with raw-text (see
> ediff-exec-process)?

Yes, tho not much worse than what we have now (where we write in
utf-8/latin-1/younameit and read it back as raw-text aka binary).
I.e. it's not a new problem.

>> > But first, we should decide whether we want such buffers to compare
>> > equal or not.
>> I believe we do, because it's called ediff-buffers.  There's ediff-files
>> for when you want to compare the files.
> Don't you think having direct file comparison yield results that are
> different from comparing buffers that visit those same files will be
> confusing?

Yes.  But I don't know of any better alternative.  Your suggestion to
emit a warning about the different encoding should hopefully make things
more clear.


        Stefan







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-19  2:17                           ` Stefan Monnier
@ 2008-10-19  7:17                             ` Eli Zaretskii
  2008-10-19  7:23                             ` Eli Zaretskii
  2008-10-19  8:32                             ` Eli Zaretskii
  2 siblings, 0 replies; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-19  7:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 1183, kifer, handa, bug-gnu-emacs, kifer

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: kifer@cs.sunysb.edu,  handa@m17n.org,  drew.adams@oracle.com,  1183@emacsbugs.donarmstrong.com,  bug-gnu-emacs@gnu.org,  kifer@cs.stonybrook.edu
> Date: Sat, 18 Oct 2008 22:17:06 -0400
> 
> >> Back then Stefan suggested emacs-mule instead of no-conversion, but for some
> >> reason this was not done--don't remember why.
> 
> > No special reason.
> 
> Actually, after looking at it a little bit, there are some reasons: the
> encoding is currently set in ediff-make-temp-file which is used in many
> different circumstances, some of which should use emacs-internal, but
> maybe not all.

We have ediff-job-name to distinguish between the different
use-cases.  Ediff already does that in several places, e.g., to pass
the "--binary" option to Diff.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-19  2:17                           ` Stefan Monnier
  2008-10-19  7:17                             ` Eli Zaretskii
@ 2008-10-19  7:23                             ` Eli Zaretskii
  2008-10-19  8:32                             ` Eli Zaretskii
  2 siblings, 0 replies; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-19  7:23 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 1183, kifer, handa, bug-gnu-emacs, kifer

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: kifer@cs.sunysb.edu,  handa@m17n.org,  drew.adams@oracle.com,  1183@emacsbugs.donarmstrong.com,  bug-gnu-emacs@gnu.org,  kifer@cs.stonybrook.edu
> Date: Sat, 18 Oct 2008 22:17:06 -0400
> 
> > The problem with that is that ediff-exec-process then uses raw-text to
> > read the output from Diff back into Emacs.
> 
> Yes, raw-text is wrong.  It should probably use undecided at least, or
> otherwise try to be more clever and use the encoding used by the
> source files.

There are 2 files involved in this, and each one can be in a different
encoding.  Thus, undecided is also not quite correct, since it will at
best most probably pick up at random one of the 2 encodings.






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-19  2:17                           ` Stefan Monnier
  2008-10-19  7:17                             ` Eli Zaretskii
  2008-10-19  7:23                             ` Eli Zaretskii
@ 2008-10-19  8:32                             ` Eli Zaretskii
  2008-10-19 15:07                               ` Drew Adams
  2 siblings, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-19  8:32 UTC (permalink / raw)
  To: drew.adams; +Cc: 1183, bug-gnu-emacs, kifer, kifer

Drew, please see if the patch below fixes the problem for you.

If you are not using a very recent CVS code, you will probably need to
use utf-8-emacs-unix instead of emacs-internal in the ediff-init.el
change, because emacs-internal was only introduced yesterday.

Michael, could you please add a warning message in buffer jobs about
differences in the values of buffer-file-coding-system between the
buffers being compared?  In particular, if there are no differences in
a region, but the above values are different, it would be good if
Ediff would say something like "only character-encoding differences"
instead of "only white-space differences".

Thanks.

2008-10-19  Eli Zaretskii  <eliz@gnu.org>

	Fix Bug #1183:

	* ediff-diff.el (ediff-exec-process): For buffer jobs, bind
	coding-system-for-read to ediff-coding-system-for-write.

	* ediff-util.el (ediff-make-temp-file): Unconditionally bind
	coding-system-for-write to ediff-coding-system-for-write.

	* ediff-init.el (ediff-coding-system-for-read): Doc fix.
	(ediff-coding-system-for-write): Set to emacs-internal.


Index: lisp/ediff-init.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/ediff-init.el,v
retrieving revision 1.93
diff -u -r1.93 ediff-init.el
--- lisp/ediff-init.el	31 Jul 2008 05:33:43 -0000	1.93
+++ lisp/ediff-init.el	19 Oct 2008 08:20:30 -0000
@@ -719,17 +719,17 @@
 
 (defcustom ediff-coding-system-for-read 'raw-text
   "*The coding system for read to use when running the diff program as a subprocess.
-In most cases, the default will do. However, under certain circumstances in
-Windows NT/98/95 you might need to use something like 'raw-text-dos here.
+In most cases, the default will do.  However, under certain circumstances in
+MS-Windows you might need to use something like 'raw-text-dos here.
 So, if the output that your diff program sends to Emacs contains extra ^M's,
 you might need to experiment here, if the default or 'raw-text-dos doesn't
 work."
   :type 'symbol
   :group 'ediff)
 
-(defcustom ediff-coding-system-for-write 'no-conversion
+(defcustom ediff-coding-system-for-write 'emacs-internal
   "*The coding system for write to use when writing out difference regions
-to temp files when Ediff needs to find fine differences."
+to temp files in buffer jobs and when Ediff needs to find fine differences."
   :type 'symbol
   :group 'ediff)
 
Index: lisp/ediff-util.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/ediff-util.el,v
retrieving revision 1.93
diff -u -r1.93 ediff-util.el
--- lisp/ediff-util.el	31 Jul 2008 05:33:43 -0000	1.93
+++ lisp/ediff-util.el	19 Oct 2008 08:20:55 -0000
@@ -3146,11 +3146,7 @@
 (defun ediff-make-temp-file (buff &optional prefix given-file start end)
   (let* ((p (ediff-convert-standard-filename (or prefix "ediff")))
 	 (short-p p)
-	 (coding-system-for-write
-	  (ediff-with-current-buffer buff
-	    (if (boundp 'buffer-file-coding-system)
-		buffer-file-coding-system
-	      ediff-coding-system-for-write)))
+	 (coding-system-for-write ediff-coding-system-for-write)
 	 f short-f)
     (if (and (fboundp 'msdos-long-file-names)
 	     (not (msdos-long-file-names))
Index: lisp/ediff-diff.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/ediff-diff.el,v
retrieving revision 1.72
diff -u -r1.72 ediff-diff.el
--- lisp/ediff-diff.el	31 Jul 2008 05:33:42 -0000	1.72
+++ lisp/ediff-diff.el	19 Oct 2008 08:21:10 -0000
@@ -1207,7 +1207,13 @@
 ;; args.
 (defun ediff-exec-process (program buffer synch options &rest files)
   (let ((data (match-data))
-	(coding-system-for-read ediff-coding-system-for-read)
+	;; If this is a buffer job, we are diffing temporary files
+	;; produced by Emacs with ediff-coding-system-for-write, so
+	;; use the same encoding to read the results.
+	(coding-system-for-read
+	 (if (string-match "buffer" (symbol-name ediff-job-name))
+	     ediff-coding-system-for-write
+	   ediff-coding-system-for-read))
 	args)
     (setq args (append (split-string options) files))
     (setq args (delete "" (delq nil args))) ; delete nil and "" from arguments






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-19  8:32                             ` Eli Zaretskii
@ 2008-10-19 15:07                               ` Drew Adams
  2008-10-19 15:32                                 ` Eli Zaretskii
  0 siblings, 1 reply; 27+ messages in thread
From: Drew Adams @ 2008-10-19 15:07 UTC (permalink / raw)
  To: 'Eli Zaretskii'; +Cc: 1183, bug-gnu-emacs, kifer, kifer

> From: Eli Zaretskii Sent: Sunday, October 19, 2008 1:33 AM
> Drew, please see if the patch below fixes the problem for you.
> 
> If you are not using a very recent CVS code, you will probably need to
> use utf-8-emacs-unix instead of emacs-internal in the ediff-init.el
> change, because emacs-internal was only introduced yesterday.
> 
> Michael, could you please add a warning message in buffer jobs about
> differences in the values of buffer-file-coding-system between the
> buffers being compared?  In particular, if there are no differences in
> a region, but the above values are different, it would be good if
> Ediff would say something like "only character-encoding differences"
> instead of "only white-space differences". Thanks.
> 2008-10-19  Eli Zaretskii  <eliz@gnu.org>
>
> 	Fix Bug #1183:
> 	* ediff-diff.el (ediff-exec-process): For buffer jobs, bind
> 	coding-system-for-read to ediff-coding-system-for-write.
> 	* ediff-util.el (ediff-make-temp-file): Unconditionally bind
> 	coding-system-for-write to ediff-coding-system-for-write.
> 	* ediff-init.el (ediff-coding-system-for-read): Doc fix.
> 	(ediff-coding-system-for-write): Set to emacs-internal.


Yes, it appears to be fixed, judging by the test case I sent.

Thank you! 







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: 23.0.60; ediff-buffers is broken
  2008-10-19 15:07                               ` Drew Adams
@ 2008-10-19 15:32                                 ` Eli Zaretskii
  0 siblings, 0 replies; 27+ messages in thread
From: Eli Zaretskii @ 2008-10-19 15:32 UTC (permalink / raw)
  To: Drew Adams; +Cc: 1183, bug-gnu-emacs, kifer, kifer

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <kifer@cs.sunysb.edu>, <1183@emacsbugs.donarmstrong.com>,
>         <bug-gnu-emacs@gnu.org>, <kifer@cs.stonybrook.edu>
> Date: Sun, 19 Oct 2008 08:07:17 -0700
> 
> > 	Fix Bug #1183:
> > 	* ediff-diff.el (ediff-exec-process): For buffer jobs, bind
> > 	coding-system-for-read to ediff-coding-system-for-write.
> > 	* ediff-util.el (ediff-make-temp-file): Unconditionally bind
> > 	coding-system-for-write to ediff-coding-system-for-write.
> > 	* ediff-init.el (ediff-coding-system-for-read): Doc fix.
> > 	(ediff-coding-system-for-write): Set to emacs-internal.
> 
> 
> Yes, it appears to be fixed, judging by the test case I sent.

Thanks for testing.  I closed the bug.






^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#1183: marked as done (23.0.60; ediff-buffers is broken)
  2008-10-16 18:47 ` bug#1183: 23.0.60; ediff-buffers is broken Drew Adams
  2008-10-16 20:25   ` Eli Zaretskii
@ 2008-10-19 15:40   ` Emacs bug Tracking System
  1 sibling, 0 replies; 27+ messages in thread
From: Emacs bug Tracking System @ 2008-10-19 15:40 UTC (permalink / raw)
  To: Eli Zaretskii

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


Your message dated Sun, 19 Oct 2008 17:32:07 +0200
with message-id <ur66ck3d4.fsf@gnu.org>
and subject line RE: bug#1183: 23.0.60; ediff-buffers is broken
has caused the Emacs bug report #1183,
regarding 23.0.60; ediff-buffers is broken
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
1183: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1183
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 3480 bytes --]

From: "Drew Adams" <drew.adams@oracle.com>
To: <emacs-pretest-bug@gnu.org>
Subject: 23.0.60; ediff-buffers is broken
Date: Thu, 16 Oct 2008 11:47:11 -0700
Message-ID: <00a101c92fbf$998d19b0$c2b22382@us.oracle.com>

emacs -Q
 
Visit both the bookmark.el file from this installation (see below) and
a copy of bookmark.el from CVS for today, 2008-10-16.
 
The only difference between the two files is in fact this text which
was added to the CVS version near the end of the file, just before
run-hooks:
 
(defun bookmark-unload-function ()
  "Unload the Bookmark library."
  (when bookmark-save-flag (bookmark-save))
  ;; continue standard unloading
  nil)
 
You can see this by using ediff in Emacs 22, 21, or 20 - or by using
diff.
 
However, ediff-buffers shows the entire buffers as a single diff, and
hitting `*' to refine that diff has no effect at all. IOW,
ediff-buffers is now useless for seeing the differences between these
two files.
 

In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
 of 2008-10-03 on LENNART-69DE564
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --no-opt --cflags -Ic:/g/include
-fno-crossjumping'
 




[-- Attachment #3: Type: message/rfc822, Size: 1160 bytes --]

From: Eli Zaretskii <eliz@gnu.org>
To: 1183-done@emacsbugs.donarmstrong.com
Subject: RE: bug#1183: 23.0.60; ediff-buffers is broken
Date: Sun, 19 Oct 2008 17:32:07 +0200
Message-ID: <ur66ck3d4.fsf@gnu.org>

Fixed by using the emacs-internal encoding when writing buffers to
temporary files.


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2008-10-19 15:40 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <ur66ck3d4.fsf@gnu.org>
2008-10-16 18:47 ` bug#1183: 23.0.60; ediff-buffers is broken Drew Adams
2008-10-16 20:25   ` Eli Zaretskii
2008-10-16 20:45     ` Drew Adams
2008-10-16 21:15       ` Eli Zaretskii
2008-10-16 21:58         ` Drew Adams
2008-10-17 12:38         ` Eli Zaretskii
2008-10-17 14:36           ` Drew Adams
2008-10-17 16:02           ` Stefan Monnier
2008-10-17 16:48             ` Drew Adams
2008-10-17 17:05               ` Michael Kifer
2008-10-17 17:17                 ` Drew Adams
2008-10-17 18:15                   ` Eli Zaretskii
2008-10-17 18:35                     ` Drew Adams
2008-10-18  3:17                       ` Michael Kifer
2008-10-18  3:43                         ` Drew Adams
2008-10-18  9:07                         ` Eli Zaretskii
2008-10-19  2:17                           ` Stefan Monnier
2008-10-19  7:17                             ` Eli Zaretskii
2008-10-19  7:23                             ` Eli Zaretskii
2008-10-19  8:32                             ` Eli Zaretskii
2008-10-19 15:07                               ` Drew Adams
2008-10-19 15:32                                 ` Eli Zaretskii
2008-10-17 18:19               ` Eli Zaretskii
2008-10-17 18:35                 ` Drew Adams
2008-10-17 18:34             ` Eli Zaretskii
2008-10-19  2:21               ` Stefan Monnier
2008-10-19 15:40   ` bug#1183: marked as done (23.0.60; ediff-buffers is broken) Emacs bug Tracking System

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).