GNU bug report logs - #25365
25.1; Coding system for bookmarks and desktop

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#25365: closed (25.1; Coding system for bookmarks and desktop)
Date: Sat, 07 Jan 2017 12:47:01 +0000

[Message part 1 (text/plain, inline)]

Your message dated Sat, 07 Jan 2017 14:46:19 +0200
with message-id <83pojzavhg.fsf <at> gnu.org>
and subject line Re: bug#25365: 25.1; Coding system for bookmarks and desktop
has caused the debbugs.gnu.org bug report #25365,
regarding 25.1; Coding system for bookmarks and desktop
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
25365: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=25365
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Dmitri Paduchikh <dpaduchikh <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; Coding system for bookmarks and desktop
Date: Thu, 05 Jan 2017 17:37:30 +0500

Hello,

It appears that all Cyrillic text in my bookmarks file has been
corrupted. I wasn't able to reproduce such a corruption using emacs -Q,
so probably this is due to interference with my settings which I will
have to investigate. But in my opinion there is a problem with
bookmark.el as well since it ignores completely coding system when
saving bookmarks. Thus I have written the following two advices to fix
its behavior. It seems that they work as needed.

(advice-add 'bookmark-write-file :around
            #'(lambda (f &rest args)
                (let ((coding-system-for-write (or coding-system-for-write 'utf-8-emacs)))
                  (apply f args)))
            '((name . "coding")))

(advice-add 'bookmark-insert-file-format-version-stamp :before
            #'(lambda (&rest args)
                (when coding-system-for-write
                  (insert (format "\
;;;; Emacs Bookmark Format Version %d ;;;;
;;; -*- coding: %S -*-\n"
                                  bookmark-file-format-version
                                  coding-system-for-write))))
            '((name . "coding")))

Besides, although desktop.el specifies coding system in its file, it is
old one - emacs-mule. Shouldn't this be utf-8-emacs these days instead?

With best regards
Dmitri Paduchikh

In GNU Emacs 25.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.5)
 of 2016-12-23 built on juergen

[Message part 3 (message/rfc822, inline)]

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitri Paduchikh <dpaduchikh <at> gmail.com>
Cc: 25365-done <at> debbugs.gnu.org
Subject: Re: bug#25365: 25.1; Coding system for bookmarks and desktop
Date: Sat, 07 Jan 2017 14:46:19 +0200

> From: Dmitri Paduchikh <dpaduchikh <at> gmail.com>
> Date: Thu, 05 Jan 2017 17:37:30 +0500
> 
> It appears that all Cyrillic text in my bookmarks file has been
> corrupted. I wasn't able to reproduce such a corruption using emacs -Q,
> so probably this is due to interference with my settings which I will
> have to investigate. But in my opinion there is a problem with
> bookmark.el as well since it ignores completely coding system when
> saving bookmarks. Thus I have written the following two advices to fix
> its behavior. It seems that they work as needed.
> 
> (advice-add 'bookmark-write-file :around
>             #'(lambda (f &rest args)
>                 (let ((coding-system-for-write (or coding-system-for-write 'utf-8-emacs)))
>                   (apply f args)))
>             '((name . "coding")))

Thanks, I used something similar in the patch below, except that I
think we should honor the existing coding cookie in the bookmark file,
instead of forcing the user to override the default each time the file
is saved.  So we should record the encoding when we load the file, and
apply it when saving.  This way, the user could specify the
non-default encoding only once, using "C-x RET c", and have that value
honored by Emacs thereafter.

> (advice-add 'bookmark-insert-file-format-version-stamp :before
>             #'(lambda (&rest args)
>                 (when coding-system-for-write
>                   (insert (format "\
> ;;;; Emacs Bookmark Format Version %d ;;;;
> ;;; -*- coding: %S -*-\n"
>                                   bookmark-file-format-version
>                                   coding-system-for-write))))
>             '((name . "coding")))

This is not quite right, as coding-system-for-write could be something
inappropriate, like undecided or prefer-utf-8.  And again, we should
honor the existing encoding.  So I modified this part to support that.

I installed the patch below on the master branch.  Please try it
(after removing your advices) and see if it gives good results.  If
you see problems, please reopen this bug with the details.

Thanks.

> Besides, although desktop.el specifies coding system in its file, it is
> old one - emacs-mule. Shouldn't this be utf-8-emacs these days instead?

I'm not sure how many users share the desktop files with older Emacs
versions that didn't support utf-8-emacs.  There's nothing wrong with
using emacs-mule in recent Emacs versions, for files that aren't
supposed to be read by programs that are not Emacs.

commit e272032769178768cf970839a9c22aba1f5b572e
Author:     Eli Zaretskii <eliz <at> gnu.org>
Date: Sat Jan 7 14:33:41 2017 +0200

    Specify encoding of the bookmark file
    
    * lisp/bookmark.el (bookmark-insert-file-format-version-stamp):
    Accept an argument CODING and include a 'coding:' cookie in the
    bookmark file preamble.
    (bookmark-upgrade-file-format-from-0): Call
    'bookmark-insert-file-format-version-stamp' with the file buffer's
    encoding, as detected when it was read.
    (bookmark-file-coding-system): New variable.
    (bookmark-load): Set bookmark-file-coding-system to the encoding
    of the loaded file.
    (bookmark-write-file): Bind coding-system-for-write to either the
    user setting via "C-x RET c" or to the existing file encoding,
    defaulting to 'utf-8-emacs'.  Update the value of
    bookmark-file-coding-system.  (Bug#25365)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 3440a52..e18362b 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -267,6 +267,8 @@ bookmark-alist
 (defvar bookmarks-already-loaded nil
   "Non-nil if and only if bookmarks have been loaded from `bookmark-default-file'.")
 
+(defvar bookmark-file-coding-system nil
+  "The coding-system of the last loaded or saved bookmark file.")
 
 ;; more stuff added by db.
 
@@ -689,7 +691,7 @@ bookmark-upgrade-file-format-from-0
   (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)
+    (bookmark-insert-file-format-version-stamp buffer-file-coding-system)
     (pp new-list (current-buffer))
     (save-buffer))
   (goto-char (point-min))
@@ -726,11 +728,14 @@ bookmark-maybe-upgrade-file-format
       (error "Bookmark file format version strangeness")))))
 
 
-(defun bookmark-insert-file-format-version-stamp ()
-  "Insert text indicating current version of bookmark file format."
+(defun bookmark-insert-file-format-version-stamp (coding)
+  "Insert text indicating current version of bookmark file format.
+CODING is the symbol of the coding-system in which the file is encoded."
+  (if (memq (coding-system-base coding) '(undecided prefer-utf-8))
+      (setq coding 'utf-8-emacs))
   (insert
-   (format ";;;; Emacs Bookmark Format Version %d ;;;;\n"
-           bookmark-file-format-version))
+   (format ";;;; Emacs Bookmark Format Version %d ;;;; -*- coding: %S -*- \n"
+           bookmark-file-format-version (coding-system-base coding)))
   (insert ";;; This format is meant to be slightly human-readable;\n"
           ";;; nevertheless, you probably don't want to edit it.\n"
           ";;; "
@@ -1417,14 +1422,17 @@ bookmark-write-file
   (with-current-buffer (get-buffer-create " *Bookmarks*")
     (goto-char (point-min))
     (delete-region (point-min) (point-max))
-    (let ((print-length nil)
+    (let ((coding-system-for-write
+           (or coding-system-for-write
+               bookmark-file-coding-system 'utf-8-emacs))
+          (print-length nil)
           (print-level nil)
           ;; See bug #12503 for why we bind `print-circle'.  Users
           ;; can define their own bookmark types, which can result in
           ;; arbitrary Lisp objects being stored in bookmark records,
           ;; and some users create objects containing circularities.
           (print-circle t))
-      (bookmark-insert-file-format-version-stamp)
+      (bookmark-insert-file-format-version-stamp coding-system-for-write)
       (insert "(")
       ;; Rather than a single call to `pp' we make one per bookmark.
       ;; Apparently `pp' has a poor algorithmic complexity, so this
@@ -1440,6 +1448,7 @@ bookmark-write-file
         (condition-case nil
             (write-region (point-min) (point-max) file)
           (file-error (message "Can't write %s" file)))
+        (setq bookmark-file-coding-system coding-system-for-write)
         (kill-buffer (current-buffer))
         (bookmark-maybe-message
          "Saving bookmarks to file %s...done" file)))))
@@ -1521,7 +1530,8 @@ bookmark-load
                     (expand-file-name bookmark-default-file))
                    file)
                   (setq bookmarks-already-loaded t))
-              (bookmark-bmenu-surreptitiously-rebuild-list))
+              (bookmark-bmenu-surreptitiously-rebuild-list)
+              (setq bookmark-file-coding-system buffer-file-coding-system))
           (error "Invalid bookmark list in %s" file)))
       (kill-buffer (current-buffer)))
     (if (null no-msg)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.