unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Dmitri Paduchikh <dpaduchikh@gmail.com>
Cc: 25365-done@debbugs.gnu.org
Subject: bug#25365: 25.1; Coding system for bookmarks and desktop
Date: Sat, 07 Jan 2017 14:46:19 +0200	[thread overview]
Message-ID: <83pojzavhg.fsf@gnu.org> (raw)
In-Reply-To: <87mvf57kdx.fsf@gmail.com> (message from Dmitri Paduchikh on Thu,  05 Jan 2017 17:37:30 +0500)

> From: Dmitri Paduchikh <dpaduchikh@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@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)





  parent reply	other threads:[~2017-01-07 12:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-05 12:37 bug#25365: 25.1; Coding system for bookmarks and desktop Dmitri Paduchikh
2017-01-06 14:56 ` Dmitri Paduchikh
2017-01-07 12:46 ` Eli Zaretskii [this message]
     [not found]   ` <87d1fxzy9d.fsf@gmail.com>
     [not found]     ` <83k2a5blo9.fsf@gnu.org>
2017-01-09 17:07       ` Dmitri Paduchikh
2017-01-09 18:49         ` Eli Zaretskii
2017-01-09 20:34           ` Dmitri Paduchikh
2017-01-10 15:49             ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83pojzavhg.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=25365-done@debbugs.gnu.org \
    --cc=dpaduchikh@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this 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).