From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Juri Linkov Newsgroups: gmane.emacs.bugs Subject: bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map Date: Thu, 03 Dec 2009 02:57:45 +0200 Organization: JURTA Message-ID: <87hbs8zx0u.fsf@mail.jurta.org> References: <87einifskr.fsf@mail.jurta.org> <87bpim8d5b.fsf@mail.jurta.org> <87fx7x5np9.fsf@mail.jurta.org> Reply-To: Juri Linkov , 5062@emacsbugs.donarmstrong.com NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1259803717 15282 80.91.229.12 (3 Dec 2009 01:28:37 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 3 Dec 2009 01:28:37 +0000 (UTC) Cc: Tassilo Horn , Brent Goodrick , 5062@emacsbugs.donarmstrong.com To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Dec 03 02:28:29 2009 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1NG0Ud-0008AF-Js for geb-bug-gnu-emacs@m.gmane.org; Thu, 03 Dec 2009 02:28:28 +0100 Original-Received: from localhost ([127.0.0.1]:37398 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NG0Ud-0003dy-53 for geb-bug-gnu-emacs@m.gmane.org; Wed, 02 Dec 2009 20:28:27 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NG0Tn-0002qY-8e for bug-gnu-emacs@gnu.org; Wed, 02 Dec 2009 20:27:35 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NG0Tg-0002hh-V9 for bug-gnu-emacs@gnu.org; Wed, 02 Dec 2009 20:27:33 -0500 Original-Received: from [199.232.76.173] (port=39778 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NG0Tf-0002gR-SP for bug-gnu-emacs@gnu.org; Wed, 02 Dec 2009 20:27:27 -0500 Original-Received: from rzlab.ucr.edu ([138.23.92.77]:48448) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NG0Tf-0005Xv-5a for bug-gnu-emacs@gnu.org; Wed, 02 Dec 2009 20:27:27 -0500 Original-Received: from rzlab.ucr.edu (rzlab.ucr.edu [127.0.0.1]) by rzlab.ucr.edu (8.14.3/8.14.3/Debian-5) with ESMTP id nB31ROAs004933; Wed, 2 Dec 2009 17:27:25 -0800 Original-Received: (from debbugs@localhost) by rzlab.ucr.edu (8.14.3/8.14.3/Submit) id nB31P5TG004649; Wed, 2 Dec 2009 17:25:05 -0800 Resent-Date: Wed, 2 Dec 2009 17:25:05 -0800 X-Loop: owner@emacsbugs.donarmstrong.com Resent-From: Juri Linkov Resent-To: bug-submit-list@donarmstrong.com Resent-CC: Emacs Bugs 2Resent-Date: Thu, 03 Dec 2009 01:25:05 +0000 Resent-Message-ID: Resent-Sender: owner@emacsbugs.donarmstrong.com X-Emacs-PR-Message: followup 5062 X-Emacs-PR-Package: emacs X-Emacs-PR-Keywords: Original-Received: via spool by 5062-submit@emacsbugs.donarmstrong.com id=B5062.12598031794206 (code B ref 5062); Thu, 03 Dec 2009 01:25:05 +0000 Original-Received: (at 5062) by emacsbugs.donarmstrong.com; 3 Dec 2009 01:19:39 +0000 X-Spam-Bayes: score:0.5 Bayes not run. spammytokens:Tokens not available. hammytokens:Tokens not available. Original-Received: from mx2.starman.ee (smtp-out4.starman.ee [85.253.0.6]) by rzlab.ucr.edu (8.14.3/8.14.3/Debian-5) with ESMTP id nB31Jahs004202 for <5062@emacsbugs.donarmstrong.com>; Wed, 2 Dec 2009 17:19:38 -0800 X-Virus-Scanned: by Amavisd-New at mx2.starman.ee Original-Received: from mail.starman.ee (62.65.209.27.cable.starman.ee [62.65.209.27]) by mx2.starman.ee (Postfix) with ESMTP id 0ABCD3F409D; Thu, 3 Dec 2009 03:19:28 +0200 (EET) In-Reply-To: (Stefan Monnier's message of "Sun, 29 Nov 2009 13:33:06 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (x86_64-pc-linux-gnu) X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 2) Resent-Date: Wed, 02 Dec 2009 20:27:33 -0500 X-BeenThere: bug-gnu-emacs@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:33161 Archived-At: >> But using the same handling of major/minor modes >> would be good. The patch below leaves the role for `image-minor-mode' >> to only provide the `C-c C-c' binding, and also toggles between >> `image-mode' and `image-mode-maybe' in `image-toggle-display' . > > I haven't looked in detail (partly because it doesn't apply to the > current code because of some other change I installed in the mean time), > but it looks good. Since feature freeze is postponed waiting for Alan's code (or maybe bug#5062 counts as a bug fix, but anyway) below is a patch to push to savannah. It refactors image-mode.el to assign clean roles to the following modes and functions: image-mode - major mode that displays a file as an image. When there are an error during mode activation (either a display does not support images, an invalid image file, etc.), it switches to normal-mode + image-minor-mode and displays an error message. It uses your latest change to exit more gracefully when the image cannot be displayed. image-minor-mode - provides the `C-c C-c' key binding to toggle image display. image-mode-maybe - finds a non-image mode in `auto-mode-alist' and activates it, also activates `image-minor-mode'. (`image-mode-maybe' is not a good name, but it's kept for backward compatibility). image-toggle-display-text - cleans the current buffer from image properties. image-toggle-display-image - puts image properties to display the image. image-toggle-display - toggles between `image-mode' and `image-mode-maybe'. In `auto-mode-alist', `image-mode-maybe' is replaced with `image-mode' to restore the old 23.1 behavior of displaying the image initially. If this is ok, I'm ready to update docstrings and commit. # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: juri@jurta.org-20091203005706-u7yip52y4ylskpuc # target_branch: http://bzr.savannah.gnu.org/r/emacs/trunk # testament_sha1: 30ead91e8155b5927110d672c076120376776503 # timestamp: 2009-12-03 02:57:15 +0200 # base_revision_id: handa@m17n.org-20091202080102-2jb3nd78wntfodbr # # Begin patch === modified file 'lisp/image-mode.el' --- lisp/image-mode.el 2009-11-28 20:45:19 +0000 +++ lisp/image-mode.el 2009-12-03 00:57:06 +0000 @@ -42,10 +42,10 @@ ;;;###autoload (push (cons (purecopy "\\.p[bpgn]m\\'") 'image-mode) auto-mode-alist) ;;;###autoload (push (cons (purecopy "\\.x[bp]m\\'") 'c-mode) auto-mode-alist) -;;;###autoload (push (cons (purecopy "\\.x[bp]m\\'") 'image-mode-maybe) auto-mode-alist) +;;;###autoload (push (cons (purecopy "\\.x[bp]m\\'") 'image-mode) auto-mode-alist) ;;;###autoload (push (cons (purecopy "\\.svgz?\\'") 'xml-mode) auto-mode-alist) -;;;###autoload (push (cons (purecopy "\\.svgz?\\'") 'image-mode-maybe) auto-mode-alist) +;;;###autoload (push (cons (purecopy "\\.svgz?\\'") 'image-mode) auto-mode-alist) ;;; Image mode window-info management. @@ -286,6 +286,9 @@ This variable is used to display the current image type in the mode line.") (make-variable-buffer-local 'image-type) +(defvar image-mode-previous-major-mode nil + "Internal variable to keep the previous non-image major mode.") + (defvar image-mode-map (let ((map (make-sparse-keymap))) (suppress-keymap map) @@ -306,7 +309,7 @@ map) "Major mode keymap for viewing images in Image mode.") -(defvar image-mode-text-map +(defvar image-minor-mode-map (let ((map (make-sparse-keymap))) (define-key map "\C-c\C-c" 'image-toggle-display) map) @@ -320,68 +323,58 @@ You can use \\\\[image-toggle-display] to toggle between display as an image and display as text." (interactive) - (kill-all-local-variables) - (setq major-mode 'image-mode) - ;; Use our own bookmarking function for images. - (set (make-local-variable 'bookmark-make-record-function) - 'image-bookmark-make-record) - - ;; Keep track of [vh]scroll when switching buffers - (image-mode-setup-winprops) - - (add-hook 'change-major-mode-hook 'image-toggle-display-text nil t) - (if (display-images-p) - (if (not (image-get-display-property)) - (image-toggle-display) - ;; Set next vars when image is already displayed but local - ;; variables were cleared by kill-all-local-variables + (condition-case err + (progn + (unless (display-images-p) + (error "Display does not support images")) + + (kill-all-local-variables) + (setq major-mode 'image-mode) + + (if (not (image-get-display-property)) + (progn + (image-toggle-display-image) + ;; If attempt to display image fails. + (if (not (image-get-display-property)) + (error "Invalid image file"))) + ;; Set next vars when image is already displayed but local + ;; variables were cleared by kill-all-local-variables + (setq cursor-type nil truncate-lines t + image-type (plist-get (cdr (image-get-display-property)) :type))) + + ;; Use our own bookmarking function for images. + (set (make-local-variable 'bookmark-make-record-function) + 'image-bookmark-make-record) + + ;; Keep track of [vh]scroll when switching buffers + (image-mode-setup-winprops) + (use-local-map image-mode-map) - (setq cursor-type nil truncate-lines t - image-type (plist-get (cdr (image-get-display-property)) :type))) - (setq image-type "text") - (use-local-map image-mode-text-map)) - (setq mode-name (format "Image[%s]" image-type)) - (run-mode-hooks 'image-mode-hook) - (if (display-images-p) - (message "%s" (concat - (substitute-command-keys - "Type \\[image-toggle-display] to view as ") - (if (image-get-display-property) - "text" "an image") ".")))) + (setq mode-name (if image-type (format "Image[%s]" image-type) "Image")) + (add-hook 'change-major-mode-hook 'image-toggle-display-text nil t) + (run-mode-hooks 'image-mode-hook) + (message "%s" (concat + (substitute-command-keys + "Type \\[image-toggle-display] to view as ") + (if (image-get-display-property) + "text" "an image") "."))) + (error + (image-mode-maybe) + (funcall + (if (called-interactively-p 'any) 'error 'message) + "Cannot display image: %s" (cdr err))))) ;;;###autoload (define-minor-mode image-minor-mode "Toggle Image minor mode. With arg, turn Image minor mode on if arg is positive, off otherwise. See the command `image-mode' for more information on this mode." - nil (:eval (format " Image[%s]" image-type)) image-mode-text-map + nil (:eval (if image-type (format " Image[%s]" image-type) " Image")) + image-minor-mode-map :group 'image :version "22.1" - (if (not image-minor-mode) - (image-toggle-display-text) - (image-mode-setup-winprops) - (add-hook 'change-major-mode-hook (lambda () (image-minor-mode -1)) nil t) - (if (display-images-p) - (condition-case err - (progn - (if (not (image-get-display-property)) - (image-toggle-display) - (setq cursor-type nil truncate-lines t - image-type (plist-get (cdr (image-get-display-property)) - :type))) - (message "%s" - (concat - (substitute-command-keys - "Type \\[image-toggle-display] to view the image as ") - (if (image-get-display-property) - "text" "an image") "."))) - (error - (image-toggle-display-text) - (funcall - (if (called-interactively-p 'any) 'error 'message) - "Cannot display image: %s" (cdr err)))) - (setq image-type "text") - (use-local-map image-mode-text-map)))) + (if image-minor-mode + (add-hook 'change-major-mode-hook (lambda () (image-minor-mode -1)) nil t))) ;;;###autoload (defun image-mode-maybe () @@ -394,94 +387,102 @@ See commands `image-mode' and `image-minor-mode' for more information on these modes." (interactive) - (let* ((mode-alist - (delq nil (mapcar - (lambda (elt) - (unless (memq (or (car-safe (cdr elt)) (cdr elt)) - '(image-mode image-mode-maybe)) - elt)) - auto-mode-alist)))) - (if (assoc-default buffer-file-name mode-alist 'string-match) - (let ((auto-mode-alist mode-alist) - (magic-mode-alist nil)) - (set-auto-mode) - (image-minor-mode t)) - (image-mode)))) + ;; image-mode-maybe = normal-mode + image-minor-mode + (let ((previous-image-type image-type)) ; preserve `image-type' + (if image-mode-previous-major-mode + ;; Restore previous major mode that was already found by this + ;; function and cached in `image-mode-previous-major-mode' + (funcall image-mode-previous-major-mode) + (let ((auto-mode-alist + (delq nil (mapcar + (lambda (elt) + (unless (memq (or (car-safe (cdr elt)) (cdr elt)) + '(image-mode image-mode-maybe)) + elt)) + auto-mode-alist))) + (magic-fallback-mode-alist + (delq nil (mapcar + (lambda (elt) + (unless (memq (or (car-safe (cdr elt)) (cdr elt)) + '(image-mode image-mode-maybe)) + elt)) + magic-fallback-mode-alist)))) + (normal-mode) + (set (make-local-variable 'image-mode-previous-major-mode) major-mode))) + (image-minor-mode 1) + (image-toggle-display-text) + ;; Restore `image-type' after `kill-all-local-variables' in `normal-mode'. + (setq image-type previous-image-type))) (defun image-toggle-display-text () "Showing the text of the image file." - (if (image-get-display-property) - (image-toggle-display))) + (let ((inhibit-read-only t) + (buffer-undo-list t) + (modified (buffer-modified-p))) + (remove-list-of-text-properties (point-min) (point-max) + '(display intangible read-nonsticky + read-only front-sticky)) + (set-buffer-modified-p modified) + (if (called-interactively-p 'any) + (message "Repeat this command to go back to displaying the image")))) (defvar archive-superior-buffer) (defvar tar-superior-buffer) (declare-function image-refresh "image.c" (spec &optional frame)) +(defun image-toggle-display-image () + "Showing the image of the image file." + ;; Turn the image data into a real image, but only if the whole file + ;; was inserted + (let* ((filename (buffer-file-name)) + (data-p (not (and filename + (file-readable-p filename) + (not (file-remote-p filename)) + (not (buffer-modified-p)) + (not (and (boundp 'archive-superior-buffer) + archive-superior-buffer)) + (not (and (boundp 'tar-superior-buffer) + tar-superior-buffer))))) + (file-or-data (if data-p + (string-make-unibyte + (buffer-substring-no-properties (point-min) (point-max))) + filename)) + (type (image-type file-or-data nil data-p)) + (image (create-image file-or-data type data-p)) + (props + `(display ,image + intangible ,image + rear-nonsticky (display intangible) + read-only t front-sticky (read-only))) + (inhibit-read-only t) + (buffer-undo-list t) + (modified (buffer-modified-p))) + (image-refresh image) + (let ((buffer-file-truename nil)) ; avoid changing dir mtime by lock_file + (add-text-properties (point-min) (point-max) props) + (restore-buffer-modified-p modified)) + ;; Inhibit the cursor when the buffer contains only an image, + ;; because cursors look very strange on top of images. + (setq cursor-type nil) + ;; This just makes the arrow displayed in the right fringe + ;; area look correct when the image is wider than the window. + (setq truncate-lines t) + ;; Allow navigation of large images + (set (make-local-variable 'auto-hscroll-mode) nil) + (setq image-type type) + (if (eq major-mode 'image-mode) + (setq mode-name (format "Image[%s]" type))) + (if (called-interactively-p 'any) + (message "Repeat this command to go back to displaying the file as text")))) + (defun image-toggle-display () "Start or stop displaying an image file as the actual image. This command toggles between showing the text of the image file and showing the image as an image." (interactive) (if (image-get-display-property) - (let ((inhibit-read-only t) - (buffer-undo-list t) - (modified (buffer-modified-p))) - (remove-list-of-text-properties (point-min) (point-max) - '(display intangible read-nonsticky - read-only front-sticky)) - (set-buffer-modified-p modified) - (kill-local-variable 'cursor-type) - (kill-local-variable 'truncate-lines) - (kill-local-variable 'auto-hscroll-mode) - (use-local-map image-mode-text-map) - (setq image-type "text") - (if (eq major-mode 'image-mode) - (setq mode-name "Image[text]")) - (if (called-interactively-p 'any) - (message "Repeat this command to go back to displaying the image"))) - ;; Turn the image data into a real image, but only if the whole file - ;; was inserted - (let* ((filename (buffer-file-name)) - (data-p (not (and filename - (file-readable-p filename) - (not (file-remote-p filename)) - (not (buffer-modified-p)) - (not (and (boundp 'archive-superior-buffer) - archive-superior-buffer)) - (not (and (boundp 'tar-superior-buffer) - tar-superior-buffer))))) - (file-or-data (if data-p - (string-make-unibyte - (buffer-substring-no-properties (point-min) (point-max))) - filename)) - (type (image-type file-or-data nil data-p)) - (image (create-image file-or-data type data-p)) - (props - `(display ,image - intangible ,image - rear-nonsticky (display intangible) - read-only t front-sticky (read-only))) - (inhibit-read-only t) - (buffer-undo-list t) - (modified (buffer-modified-p))) - (image-refresh image) - (let ((buffer-file-truename nil)) ; avoid changing dir mtime by lock_file - (add-text-properties (point-min) (point-max) props) - (restore-buffer-modified-p modified)) - ;; Inhibit the cursor when the buffer contains only an image, - ;; because cursors look very strange on top of images. - (setq cursor-type nil) - ;; This just makes the arrow displayed in the right fringe - ;; area look correct when the image is wider than the window. - (setq truncate-lines t) - ;; Allow navigation of large images - (set (make-local-variable 'auto-hscroll-mode) nil) - (use-local-map image-mode-map) - (setq image-type type) - (if (eq major-mode 'image-mode) - (setq mode-name (format "Image[%s]" type))) - (if (called-interactively-p 'any) - (message "Repeat this command to go back to displaying the file as text"))))) + (image-mode-maybe) + (image-mode))) ;;; Support for bookmark.el (declare-function bookmark-make-record-default "bookmark" -- Juri Linkov http://www.jurta.org/emacs/