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: Sun, 29 Nov 2009 18:03:50 +0200 Organization: JURTA Message-ID: <87fx7x5np9.fsf@mail.jurta.org> References: <87einifskr.fsf@mail.jurta.org> <87bpim8d5b.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 1259512066 23265 80.91.229.12 (29 Nov 2009 16:27:46 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 29 Nov 2009 16:27:46 +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 Sun Nov 29 17:27:38 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 1NEmcb-0006Y2-Rs for geb-bug-gnu-emacs@m.gmane.org; Sun, 29 Nov 2009 17:27:38 +0100 Original-Received: from localhost ([127.0.0.1]:47041 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NEmca-0003W9-Tu for geb-bug-gnu-emacs@m.gmane.org; Sun, 29 Nov 2009 11:27:36 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NEmcV-0003Un-L0 for bug-gnu-emacs@gnu.org; Sun, 29 Nov 2009 11:27:31 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NEmcQ-0003R1-PX for bug-gnu-emacs@gnu.org; Sun, 29 Nov 2009 11:27:30 -0500 Original-Received: from [199.232.76.173] (port=60209 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NEmcQ-0003Qh-Ha for bug-gnu-emacs@gnu.org; Sun, 29 Nov 2009 11:27:26 -0500 Original-Received: from rzlab.ucr.edu ([138.23.92.77]:57121) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NEmcP-0006PA-Qy for bug-gnu-emacs@gnu.org; Sun, 29 Nov 2009 11:27:26 -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 nATGRNde019804; Sun, 29 Nov 2009 08:27:23 -0800 Original-Received: (from debbugs@localhost) by rzlab.ucr.edu (8.14.3/8.14.3/Submit) id nATGF6FE018710; Sun, 29 Nov 2009 08:15:06 -0800 Resent-Date: Sun, 29 Nov 2009 08:15:06 -0800 X-Loop: owner@emacsbugs.donarmstrong.com Resent-From: Juri Linkov Resent-To: bug-submit-list@donarmstrong.com Resent-CC: Emacs Bugs 2Resent-Date: Sun, 29 Nov 2009 16:15:06 +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.125951085818125 (code B ref 5062); Sun, 29 Nov 2009 16:15:06 +0000 Original-Received: (at 5062) by emacsbugs.donarmstrong.com; 29 Nov 2009 16:07:38 +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 nATG7aHL018117 for <5062@emacsbugs.donarmstrong.com>; Sun, 29 Nov 2009 08:07:37 -0800 X-Virus-Scanned: by Amavisd-New at mx2.starman.ee Original-Received: from mail.starman.ee (82.131.68.144.cable.starman.ee [82.131.68.144]) by mx2.starman.ee (Postfix) with ESMTP id 8A9F73F40C9; Sun, 29 Nov 2009 18:07:30 +0200 (EET) In-Reply-To: (Stefan Monnier's message of "Sun, 29 Nov 2009 10:36:59 -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: Sun, 29 Nov 2009 11:27:30 -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:33034 Archived-At: >>> Better would be to save the major mode, and then switch to >>> "saved-major-mode + image-minor-mode". No need to modify anybody's >>> keymap. And I think that image-minor-mode should only be used while >>> displaying text: basically, image-toggle-display should toggle between >>> image-mode and the other major-mode (complemented with >>> image-minor-mode). >>> That would be cleaner, >> I see now how this is implemented with `doc-view-previous-major-mode' >> in doc-view. Perhaps we should "sync" this with image-mode to use >> the same logic. > > Yes. Those two modes really need to be made more alike (ideally, they > should be merged, tho there might be some good reasons to keep some of > it separate). Even though doc-view is a superset of image-mode, currently I see no way to merge them cleanly. 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' . If this is the right direction, I'd finish refactoring of image.el (some variable settings could be moved from `image-toggle-display' to `image-mode', etc.) One open question is about using `image-mode-maybe'. With this patch, it's essentially a combination of a non-image major mode (`normal-mode' with image-mode entries removed from `auto-mode-alist') and `image-minor-mode'. So when `image-mode-maybe' is present in `auto-mode-alist' this means "to set a non-image major mode and image-minor-mode". It would be more nice to get rid of `image-mode-maybe', and to use `image-minor-mode' in `auto-mode-alist'. But I don't know how to specify both a non-image major mode and image-minor-mode at the same time using `auto-mode-alist'. The approach currently used in doc-view for PS files is more ugly than using `image-mode-maybe'. It adds to ps-mode.el the line: (declare-function doc-view-minor-mode "doc-view") and calls `(doc-view-minor-mode 1)' at the end of `ps-mode'. I think this should be changed to use the solution described above adding e.g. `doc-view-mode-maybe' (or a better name) as a combination of a non-image major mode and image minor-mode. Index: lisp/image-mode.el =================================================================== RCS file: /sources/emacs/emacs/lisp/image-mode.el,v retrieving revision 1.59 diff -u -w -b -C 2 -r1.59 image-mode.el cvs diff: conflicting specifications of output style *** lisp/image-mode.el 28 Nov 2009 20:45:22 -0000 1.59 --- lisp/image-mode.el 29 Nov 2009 16:02:54 -0000 *************** *** 287,290 **** --- 287,293 ---- (make-variable-buffer-local 'image-type) + (defvar image-mode-previous-major-mode nil + "Internal variable to save the major mode.") + (defvar image-mode-map (let ((map (make-sparse-keymap))) *************** *** 307,311 **** "Major mode keymap for viewing images in Image mode.") ! (defvar image-mode-text-map (let ((map (make-sparse-keymap))) (define-key map "\C-c\C-c" 'image-toggle-display) --- 310,314 ---- "Major mode keymap for viewing images in Image mode.") ! (defvar image-minor-mode-map (let ((map (make-sparse-keymap))) (define-key map "\C-c\C-c" 'image-toggle-display) *************** *** 323,335 **** (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) --- 326,335 ---- (kill-all-local-variables) (setq major-mode 'image-mode) ! (unless (display-images-p) ! (setq image-type "text") ! (image-toggle-display-text) ! (error "Cannot display image")) (if (not (image-get-display-property)) (image-toggle-display) *************** *** 339,352 **** (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") ".")))) ;;;###autoload --- 339,358 ---- (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) ! ! (add-hook 'change-major-mode-hook 'image-toggle-display-text nil t) (setq mode-name (format "Image[%s]" image-type)) (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") "."))) ;;;###autoload *************** *** 355,386 **** 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 :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)))) ;;;###autoload --- 361,371 ---- 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 (if image-type (format " Image[%s]" image-type) " Image")) ! image-minor-mode-map :group 'image :version "22.1" ! (if image-minor-mode (add-hook 'change-major-mode-hook (lambda () (image-minor-mode -1)) nil t) ! (set (make-local-variable 'image-mode-previous-major-mode) major-mode))) ;;;###autoload *************** *** 395,399 **** information on these modes." (interactive) ! (let* ((mode-alist (delq nil (mapcar (lambda (elt) --- 380,387 ---- information on these modes." (interactive) ! ;; image-mode-maybe = normal-mode + image-minor-mode ! (if image-mode-previous-major-mode ! (funcall image-mode-previous-major-mode) ! (let ((auto-mode-alist (delq nil (mapcar (lambda (elt) *************** *** 401,411 **** '(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)))) (defun image-toggle-display-text () --- 389,405 ---- '(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)) (defun image-toggle-display-text () *************** *** 431,441 **** 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"))) --- 425,430 ---- read-only front-sticky)) (set-buffer-modified-p modified) (setq image-type "text") ! (image-mode-maybe) (if (called-interactively-p 'any) (message "Repeat this command to go back to displaying the image"))) *************** *** 477,482 **** ;; 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))) --- 466,471 ---- ;; Allow navigation of large images (set (make-local-variable 'auto-hscroll-mode) nil) (setq image-type type) + (image-mode) (if (eq major-mode 'image-mode) (setq mode-name (format "Image[%s]" type))) -- Juri Linkov http://www.jurta.org/emacs/