* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map @ 2009-11-28 0:44 ` Brent Goodrick 2009-11-28 2:25 ` Stefan Monnier 2009-12-04 22:00 ` bug#5062: marked as done (23.1.50; image-toggle-display overwrites nxml-mode local key map) Emacs bug Tracking System 0 siblings, 2 replies; 22+ messages in thread From: Brent Goodrick @ 2009-11-28 0:44 UTC (permalink / raw) To: emacs-pretest-bug Please write in English if possible, because the Emacs maintainers usually do not have translators to read other languages for them. Your bug report will be posted to the emacs-pretest-bug@gnu.org mailing list. Please describe exactly what actions triggered the bug and the precise symptoms of the bug. If you can, give a recipe starting from `emacs -Q': Insure that you have Emacs 23 built on Debian Squeeze Linux with librsvg2 library (or similar OS's and libraries), just to get the image-mode to interact with nxml-mode in its keybindings for a .svg file which is an XML derivative for SVG files. Then proceed as follows: 1. Store the following XML content into a file called /tmp/test.svg --- cut below this line --- <?xml version="1.0" standalone="no"?> <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> <svg width="400px" height="400px" viewBox="0 0 4000 4000" xmlns="http://www.w3.org/2000/svg" version="1.1"> <title>Sample Title</title> <desc>Sample Description</desc> <g transform="translate(200,200)"> <rect x="0" y="0" width="400" height="400" fill="none" stroke="blue" stroke-width="10px"/> <g transform="translate(50,25)"> <text x="0" y="0" fill="white" stroke="none">10px</text> </g> </g> <g transform="translate(200,1000)"> <rect x="0" y="0" width="400" height="400" fill="none" stroke="green" stroke-width="10px"/> <g transform="translate(50,25)"> <text x="0" y="0" fill="white" stroke="none">1px</text> </g> </g> </svg> --- cut above this line --- 2. Run emacs -Q and wait for it to load and map into the display. 3. Type C-x C-f /tmp/test.svg and see that the image of the file is displayed. 4. Type C-c C-c and note that the XML is shown. All correct behavior so far. 5. Type C-h k C-M-n and notice that the key for C-M-n is bound to `forward-list' which is not correct because the .svg file is a xml file, that, by default, should be bound to `nxml-forward-element' by the defvar for nxml-mode-map inside share/emacs/23.1.50/lisp/nxml/nxml-mode.el.gz. Notice also that nxml-mode applies its own local map with this call inside the `nxml-mode' function: (use-local-map nxml-mode-map) But compare that with `image-toggle-display' where it either calls: (use-local-map image-mode-text-map) or (use-local-map image-mode-map) Neither of which respects the nxml-mode's bindings. For validation, you can inject a "trampoline" function on `use-local-map' to show who overwrites the local map, as follows: (progn (defun xx-new-use-local-map (&rest args) (message "xx-new-use-local-map called from:\n") (backtrace) (apply xx-orig-use-local-map args)) (when (not (boundp 'xx-orig-use-local-map)) (setq xx-orig-use-local-map (symbol-function 'use-local-map)) (fset 'use-local-map 'xx-new-use-local-map))) where the "xx-*" functions were arbitrary for this bug report. Once xx-new-use-local-map is injected as given above, then use C-h C-e to see what happens when you first execute C-x C-f /tmp/test.svg, and then see it again when hitting C-c C-c while in the test.svg buffer. You will see that image-mode function obliterates the local map set by the `nxml-mode' function. I don't know what the correct solution here should be, and many questions arise: a. Should image-mode be a minor mode of nxml-mode, or, b. Should `image-mode' stash a copy of the current buffers (current-local-map), and restore it afterwards, but also applying the special binding for C-c C-c, or, c. Should `image-mode' make the other modes local map be the parent of its own local map? I'm thinking (c)? Thanks to whomever added SVG capabilities to Emacs! bgoodr If Emacs crashed, and you have the Emacs process in the gdb debugger, please include the output from the following gdb commands: `bt full' and `xbacktrace'. For information about debugging Emacs, please read the file /home/brentg/install/Linux.x86_64/share/emacs/23.1.50/etc/DEBUG. Emacs did not crash. In GNU Emacs 23.1.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.18.2) of 2009-10-31 on hungover Windowing system distributor `The X.Org Foundation', version 11.0.10605000 configured using `configure '--with-x-toolkit' '--with-xft' '--prefix=/home/brentg/install/Linux.x86_64'' Important settings: value of $LC_ALL: nil value of $LC_COLLATE: nil value of $LC_CTYPE: nil value of $LC_MESSAGES: nil value of $LC_MONETARY: nil value of $LC_NUMERIC: nil value of $LC_TIME: nil value of $LANG: en_US.UTF-8 value of $XMODIFIERS: nil locale-coding-system: utf-8-unix default enable-multibyte-characters: t Major mode: nXML Minor modes in effect: image-minor-mode: t tooltip-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t global-auto-composition-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Recent input: C-x C-f C-v / t m p / C-x h <backspace> C-b C-b C-b C-g C-x C-f C-SPC M-b C-b <backspace> / t m p C-d C-d C-d C-d C-d / t e s t . s v g <return> C-v <help-echo> <help-echo> <help-echo> <down-mouse-2> <mouse-2> C-x C-s C-x C-v <return> C-c C-c C-h k C-M-n M-x r e p o r t - e m <tab> <return> Recent messages: File mode specification error: (error "Cannot determine image type") call-interactively: End of buffer Mark set Saving file /tmp/test.svg... Wrote /tmp/test.svg Using vacuous schema Type C-c C-c to view the image as text. Repeat this command to go back to displaying the image Type C-x 1 to delete the help window. Load-path shadows: None found. Features: (shadow mail-extr message ecomplete rfc822 mml mml-sec password-cache mm-decode mm-bodies mm-encode mailcap mail-parse rfc2231 rfc2047 rfc2045 qp ietf-drums mailabbrev nnheader gnus-util netrc time-date mm-util mail-prsvr gmm-utils wid-edit mailheader canlock sha1 hex-util hashcash mail-utils emacsbug sendmail help-fns help-mode view nxml-uchnm rng-xsd xsd-regexp rng-cmpct regexp-opt rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns easymenu nxml-mode nxml-outln nxml-rap nxml-util nxml-glyph nxml-enc xmltok cl cl-19 jka-compr image-mode tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image fringe lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar mldrag mouse jit-lock font-lock syntax facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev loaddefs button minibuffer faces cus-face text-properties overlay md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote make-network-process dbusbind gtk x-toolkit x multi-tty emacs) ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-28 0:44 ` bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map Brent Goodrick @ 2009-11-28 2:25 ` Stefan Monnier 2009-11-28 15:26 ` Brent Goodrick 2009-11-28 17:49 ` Juri Linkov 2009-12-04 22:00 ` bug#5062: marked as done (23.1.50; image-toggle-display overwrites nxml-mode local key map) Emacs bug Tracking System 1 sibling, 2 replies; 22+ messages in thread From: Stefan Monnier @ 2009-11-28 2:25 UTC (permalink / raw) To: Brent Goodrick; +Cc: 5062 > 2. Run emacs -Q and wait for it to load and map into the display. > 3. Type C-x C-f /tmp/test.svg and see that the image of the file is > displayed. > 4. Type C-c C-c and note that the XML is shown. All correct behavior > so far. > 5. Type C-h k C-M-n and notice that the key for C-M-n is bound to > `forward-list' which is not correct because the .svg file is a xml Indeed, SVG files should be handled like postscript files, i.e. use image-minor-mode rather than image-mode. Can someone figure out how to to do that? Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-28 2:25 ` Stefan Monnier @ 2009-11-28 15:26 ` Brent Goodrick 2009-11-28 17:49 ` Juri Linkov 1 sibling, 0 replies; 22+ messages in thread From: Brent Goodrick @ 2009-11-28 15:26 UTC (permalink / raw) To: Stefan Monnier; +Cc: 5062 On Fri, Nov 27, 2009 at 6:25 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote: > > > 2. Run emacs -Q and wait for it to load and map into the display. > > 3. Type C-x C-f /tmp/test.svg and see that the image of the file is > > displayed. > > 4. Type C-c C-c and note that the XML is shown. All correct behavior > > so far. > > 5. Type C-h k C-M-n and notice that the key for C-M-n is bound to > > `forward-list' which is not correct because the .svg file is a xml > > Indeed, SVG files should be handled like postscript files, i.e. use > image-minor-mode rather than image-mode. > Can someone figure out how to to do that? Hmmm, seems that image-minor-mode is enabled in this case, with a major mode of nxml-mode. But image-minor-mode ultimately calls image-toggle-display, and image-toggle-display is calling use-local-map to obliterate whatever map is already there. image-minor-mode can also call use-local-map, too. All of that arrangement seems to be set up by the image-mode-maybe function. I wonder what the correct "policy" is for a minor mode w.r.t. keybindings that can shadow a major mode? Should the major mode's keymap be the top-most local keymap, with all minor modes as parent maps of that top-most local keymap? Or is it the other way around, with each minor mode "pushing" its own local mode map to be top-most, and causing the current major modes local map to be that parent of that new one that was "pushed"? bg ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-28 2:25 ` Stefan Monnier 2009-11-28 15:26 ` Brent Goodrick @ 2009-11-28 17:49 ` Juri Linkov 2009-11-28 20:21 ` Stefan Monnier 1 sibling, 1 reply; 22+ messages in thread From: Juri Linkov @ 2009-11-28 17:49 UTC (permalink / raw) To: Stefan Monnier; +Cc: Brent Goodrick, 5062 > Indeed, SVG files should be handled like postscript files, i.e. use > image-minor-mode rather than image-mode. Currently PS files are handled by DocView, but there is the same problem for .xbm/.xpm files that use c-mode. > Can someone figure out how to to do that? The problem is in `(use-local-map image-mode-text-map)'. After switching to "text" mode, it sets the local map to image-mode-text-map with a single key binding `C-c C-c' instead of restoring the original map of the major mode. The following patch fixes this bug. It saves a copy of the original map to the internal buffer-local variable `image-mode-local-map' with the additional image-mode specific key binding `C-c C-c' to switch back to the image mode. And on switching to the text mode restores the original major mode map from this variable. Index: lisp/image-mode.el =================================================================== RCS file: /sources/emacs/emacs/lisp/image-mode.el,v retrieving revision 1.58 diff -u -r1.58 image-mode.el --- lisp/image-mode.el 11 Nov 2009 05:49:13 -0000 1.58 +++ lisp/image-mode.el 28 Nov 2009 17:48:00 -0000 @@ -306,11 +306,8 @@ map) "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) - map) - "Major mode keymap for viewing images as text in Image mode.") +(defvar image-mode-local-map nil) +(make-variable-buffer-local 'image-mode-local-map) (defvar bookmark-make-record-function) @@ -329,6 +326,9 @@ ;; Keep track of [vh]scroll when switching buffers (image-mode-setup-winprops) + (setq image-mode-local-map (copy-keymap (current-local-map))) + (define-key image-mode-local-map "\C-c\C-c" 'image-toggle-display) + (add-hook 'change-major-mode-hook 'image-toggle-display-text nil t) (if (display-images-p) (if (not (image-get-display-property)) @@ -339,7 +339,7 @@ (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)) + (use-local-map image-mode-local-map)) (setq mode-name (format "Image[%s]" image-type)) (run-mode-hooks 'image-mode-hook) (if (display-images-p) @@ -354,12 +354,16 @@ "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 (format " Image[%s]" image-type)) nil :group 'image :version "22.1" (if (not image-minor-mode) (image-toggle-display-text) (image-mode-setup-winprops) + + (setq image-mode-local-map (copy-keymap (current-local-map))) + (define-key image-mode-local-map "\C-c\C-c" 'image-toggle-display) + (add-hook 'change-major-mode-hook (lambda () (image-minor-mode -1)) nil t) (if (display-images-p) (if (not (image-get-display-property)) @@ -367,7 +371,7 @@ (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)) + (use-local-map image-mode-local-map)) (if (display-images-p) (message "%s" (concat (substitute-command-keys @@ -425,7 +429,7 @@ (kill-local-variable 'cursor-type) (kill-local-variable 'truncate-lines) (kill-local-variable 'auto-hscroll-mode) - (use-local-map image-mode-text-map) + (use-local-map image-mode-local-map) (setq image-type "text") (if (eq major-mode 'image-mode) (setq mode-name "Image[text]")) -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-28 17:49 ` Juri Linkov @ 2009-11-28 20:21 ` Stefan Monnier 2009-11-28 22:54 ` Juri Linkov 0 siblings, 1 reply; 22+ messages in thread From: Stefan Monnier @ 2009-11-28 20:21 UTC (permalink / raw) To: Juri Linkov; +Cc: Brent Goodrick, 5062 > After switching to "text" mode, it sets the local map to > image-mode-text-map with a single key binding `C-c C-c' > instead of restoring the original map of the major mode. > The following patch fixes this bug. It saves a copy of the original > map to the internal buffer-local variable `image-mode-local-map' > with the additional image-mode specific key binding `C-c C-c' to > switch back to the image mode. And on switching to the text mode > restores the original major mode map from this variable. 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, Stefan > Index: lisp/image-mode.el > =================================================================== > RCS file: /sources/emacs/emacs/lisp/image-mode.el,v > retrieving revision 1.58 > diff -u -r1.58 image-mode.el > --- lisp/image-mode.el 11 Nov 2009 05:49:13 -0000 1.58 > +++ lisp/image-mode.el 28 Nov 2009 17:48:00 -0000 > @@ -306,11 +306,8 @@ > map) > "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) > - map) > - "Major mode keymap for viewing images as text in Image mode.") > +(defvar image-mode-local-map nil) > +(make-variable-buffer-local 'image-mode-local-map) > (defvar bookmark-make-record-function) > @@ -329,6 +326,9 @@ > ;; Keep track of [vh]scroll when switching buffers > (image-mode-setup-winprops) > + (setq image-mode-local-map (copy-keymap (current-local-map))) > + (define-key image-mode-local-map "\C-c\C-c" 'image-toggle-display) > + > (add-hook 'change-major-mode-hook 'image-toggle-display-text nil t) > (if (display-images-p) > (if (not (image-get-display-property)) > @@ -339,7 +339,7 @@ > (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)) > + (use-local-map image-mode-local-map)) > (setq mode-name (format "Image[%s]" image-type)) > (run-mode-hooks 'image-mode-hook) > (if (display-images-p) > @@ -354,12 +354,16 @@ > "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 (format " Image[%s]" image-type)) nil > :group 'image > :version "22.1" > (if (not image-minor-mode) > (image-toggle-display-text) > (image-mode-setup-winprops) > + > + (setq image-mode-local-map (copy-keymap (current-local-map))) > + (define-key image-mode-local-map "\C-c\C-c" 'image-toggle-display) > + > (add-hook 'change-major-mode-hook (lambda () (image-minor-mode -1)) nil t) > (if (display-images-p) > (if (not (image-get-display-property)) > @@ -367,7 +371,7 @@ > (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)) > + (use-local-map image-mode-local-map)) > (if (display-images-p) > (message "%s" (concat > (substitute-command-keys > @@ -425,7 +429,7 @@ > (kill-local-variable 'cursor-type) > (kill-local-variable 'truncate-lines) > (kill-local-variable 'auto-hscroll-mode) > - (use-local-map image-mode-text-map) > + (use-local-map image-mode-local-map) > (setq image-type "text") > (if (eq major-mode 'image-mode) > (setq mode-name "Image[text]")) > -- > Juri Linkov > http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-28 20:21 ` Stefan Monnier @ 2009-11-28 22:54 ` Juri Linkov 2009-11-29 15:36 ` Stefan Monnier 0 siblings, 1 reply; 22+ messages in thread From: Juri Linkov @ 2009-11-28 22:54 UTC (permalink / raw) To: Stefan Monnier; +Cc: Brent Goodrick, 5062 > 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. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-28 22:54 ` Juri Linkov @ 2009-11-29 15:36 ` Stefan Monnier 2009-11-29 16:03 ` Juri Linkov 0 siblings, 1 reply; 22+ messages in thread From: Stefan Monnier @ 2009-11-29 15:36 UTC (permalink / raw) To: Juri Linkov; +Cc: Brent Goodrick, 5062 >> 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). Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-29 15:36 ` Stefan Monnier @ 2009-11-29 16:03 ` Juri Linkov 2009-11-29 18:33 ` Stefan Monnier 0 siblings, 1 reply; 22+ messages in thread From: Juri Linkov @ 2009-11-29 16:03 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tassilo Horn, Brent Goodrick, 5062 >>> 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-29 16:03 ` Juri Linkov @ 2009-11-29 18:33 ` Stefan Monnier 2009-11-29 22:00 ` Lennart Borgman 2009-12-03 0:57 ` bug#5062: 23.1.50; " Juri Linkov 0 siblings, 2 replies; 22+ messages in thread From: Stefan Monnier @ 2009-11-29 18:33 UTC (permalink / raw) To: Juri Linkov; +Cc: Tassilo Horn, Brent Goodrick, 5062 > Even though doc-view is a superset of image-mode, currently I see no way > to merge them cleanly. As mentioned, it may not be easy or even desirable to merge them completely. But we should strive to keep them as close as possible and to make doc-view use image-mode.el functions and data wherever possible. Maybe if one cannot be a derived mode of them other, we could make a "shared parent" mode (like an abstract class). > 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. > 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'. IIRC auto-mode-alist can point to minor-modes (the auto-mode-alist entry needs to look something like ("regexp" minor-mode t)), but I don't think it can be used here (the constraints on how it can be used are very strict since it needs to "consume" a suffix and be called before the major-mode). Maybe we could extend auto-mode-alist to allow things like ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)). > 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") To the extent that postscript would always use image-minor-mode anyway, I think the way it's done now is at least as clean if not cleaner than relying on image-mode-maybe (image-mode-maybe is fairly hackish). > 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. I don't think that would be an improvement. This is in contrast to XPM and SVG formats where they get combined with major-modes that are also used for non-image documents, so it wouldn't make sense for nxml-mode or c-mode to always enable image-minor-mode. Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-29 18:33 ` Stefan Monnier @ 2009-11-29 22:00 ` Lennart Borgman 2009-11-29 22:08 ` Juri Linkov 2009-12-03 0:57 ` bug#5062: 23.1.50; " Juri Linkov 1 sibling, 1 reply; 22+ messages in thread From: Lennart Borgman @ 2009-11-29 22:00 UTC (permalink / raw) To: Stefan Monnier, 5062; +Cc: Tassilo Horn, Brent Goodrick On Sun, Nov 29, 2009 at 7:33 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote: > > IIRC auto-mode-alist can point to minor-modes (the auto-mode-alist entry > needs to look something like ("regexp" minor-mode t)), but I don't think > it can be used here (the constraints on how it can be used are very > strict since it needs to "consume" a suffix and be called before the > major-mode). Maybe we could extend auto-mode-alist to allow things like > ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)). Or just define a derived mode that does major-mode + minor-mode and use that in auto-mode-alist? ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-29 22:00 ` Lennart Borgman @ 2009-11-29 22:08 ` Juri Linkov 2009-11-29 23:16 ` Lennart Borgman 0 siblings, 1 reply; 22+ messages in thread From: Juri Linkov @ 2009-11-29 22:08 UTC (permalink / raw) To: Lennart Borgman; +Cc: Tassilo Horn, Brent Goodrick, 5062 >> IIRC auto-mode-alist can point to minor-modes (the auto-mode-alist entry >> needs to look something like ("regexp" minor-mode t)), but I don't think >> it can be used here (the constraints on how it can be used are very >> strict since it needs to "consume" a suffix and be called before the >> major-mode). Maybe we could extend auto-mode-alist to allow things like >> ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)). > > Or just define a derived mode that does major-mode + minor-mode and > use that in auto-mode-alist? That's what I did in the previous patch where `image-mode-maybe' is a combination of a non-image major mode (`normal-mode' with image-mode entries removed from `auto-mode-alist') and `image-minor-mode'. Or did you mean a joint mode like `c-mode-and-image-minor-mode', `nxml-mode-and-image-minor-mode', `ps-mode-and-doc-view-minor-mode'? Wouldn't this be too clumsy? -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-29 22:08 ` Juri Linkov @ 2009-11-29 23:16 ` Lennart Borgman 2009-12-03 0:59 ` bug#5062: " Juri Linkov 0 siblings, 1 reply; 22+ messages in thread From: Lennart Borgman @ 2009-11-29 23:16 UTC (permalink / raw) To: Juri Linkov; +Cc: Tassilo Horn, Brent Goodrick, 5062 On Sun, Nov 29, 2009 at 11:08 PM, Juri Linkov <juri@jurta.org> wrote: >> >> Or just define a derived mode that does major-mode + minor-mode and >> use that in auto-mode-alist? > > That's what I did in the previous patch where `image-mode-maybe' is > a combination of a non-image major mode (`normal-mode' with image-mode > entries removed from `auto-mode-alist') and `image-minor-mode'. > > Or did you mean a joint mode like `c-mode-and-image-minor-mode', > `nxml-mode-and-image-minor-mode', `ps-mode-and-doc-view-minor-mode'? > Wouldn't this be too clumsy? Yes, why would it be too clumsy? A more flexibel way might be to use define-globalized-minor-mode. The turn on function there could make any check. It could for example look in a list similar to auto-mode-alist, but for minor modes. But maybe that would take too long time? ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: image-toggle-display overwrites nxml-mode local key map 2009-11-29 23:16 ` Lennart Borgman @ 2009-12-03 0:59 ` Juri Linkov 2009-12-03 1:37 ` Lennart Borgman 0 siblings, 1 reply; 22+ messages in thread From: Juri Linkov @ 2009-12-03 0:59 UTC (permalink / raw) To: Lennart Borgman; +Cc: Tassilo Horn, Brent Goodrick, 5062 >> Or did you mean a joint mode like `c-mode-and-image-minor-mode', >> `nxml-mode-and-image-minor-mode', `ps-mode-and-doc-view-minor-mode'? >> Wouldn't this be too clumsy? > > Yes, why would it be too clumsy? > > A more flexibel way might be to use define-globalized-minor-mode. The > turn on function there could make any check. It could for example look > in a list similar to auto-mode-alist, but for minor modes. > > But maybe that would take too long time? I think Stefan's idea of allowing auto-mode-alist to have entries like ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)) is more universal. Two basic ways to specify modes are: 1. With Local Variables you can put in the first line -*- mode: major-mode; mode: minor-mode-1; mode: minor-mode-2; ... -*- or the same to the Local Variables list, or to the Directory Local Variables. 2. With `auto-mode-alist' you can bind filename patterns to major modes, but not to minor modes. (major-mode minor-mode-1 minor-mode-2 ...) would allow to do the same that Local Variables already allows to do, e.g. ("\\.xpm\\'" (c-mode image-minor-mode)), ("\\.svg\\'" (nxml-mode image-minor-mode)), ("\\.ps\\'" (ps-mode doc-view-minor-mode)), etc. PS: this feature currently is not too necessary, because in the patch I sent `image-mode-maybe' was replaced with `image-mode' in `auto-mode-alist' to display a file as an image by default (the default behavior in 23.1). And only if someone wants to change this default to display a file as text initially, this requires either adding `image-mode-maybe' to `auto-mode-alist' or a combination of normal-mode + image-minor-mode that can be implemented as a new feature. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: image-toggle-display overwrites nxml-mode local key map 2009-12-03 0:59 ` bug#5062: " Juri Linkov @ 2009-12-03 1:37 ` Lennart Borgman 2009-12-03 3:08 ` Kevin Rodgers 2009-12-03 3:30 ` Stefan Monnier 0 siblings, 2 replies; 22+ messages in thread From: Lennart Borgman @ 2009-12-03 1:37 UTC (permalink / raw) To: Juri Linkov; +Cc: Tassilo Horn, Brent Goodrick, 5062 On Thu, Dec 3, 2009 at 1:59 AM, Juri Linkov <juri@jurta.org> wrote: >>> Or did you mean a joint mode like `c-mode-and-image-minor-mode', >>> `nxml-mode-and-image-minor-mode', `ps-mode-and-doc-view-minor-mode'? >>> Wouldn't this be too clumsy? >> >> Yes, why would it be too clumsy? >> >> A more flexibel way might be to use define-globalized-minor-mode. The >> turn on function there could make any check. It could for example look >> in a list similar to auto-mode-alist, but for minor modes. >> >> But maybe that would take too long time? > > I think Stefan's idea of allowing auto-mode-alist to have entries like > ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)) is more universal. Why not allow a form there then: ("regexp" '(progn (major-mode) (mino-mode-1 1) (minor-mode-2 1) ...)) ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: image-toggle-display overwrites nxml-mode local key map 2009-12-03 1:37 ` Lennart Borgman @ 2009-12-03 3:08 ` Kevin Rodgers 2009-12-03 3:31 ` Lennart Borgman 2009-12-03 3:30 ` Stefan Monnier 1 sibling, 1 reply; 22+ messages in thread From: Kevin Rodgers @ 2009-12-03 3:08 UTC (permalink / raw) To: bug-gnu-emacs Lennart Borgman wrote: > On Thu, Dec 3, 2009 at 1:59 AM, Juri Linkov <juri@jurta.org> wrote: >>>> Or did you mean a joint mode like `c-mode-and-image-minor-mode', >>>> `nxml-mode-and-image-minor-mode', `ps-mode-and-doc-view-minor-mode'? >>>> Wouldn't this be too clumsy? >>> Yes, why would it be too clumsy? >>> >>> A more flexibel way might be to use define-globalized-minor-mode. The >>> turn on function there could make any check. It could for example look >>> in a list similar to auto-mode-alist, but for minor modes. >>> >>> But maybe that would take too long time? >> I think Stefan's idea of allowing auto-mode-alist to have entries like >> ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)) is more universal. > > > Why not allow a form there then: > > ("regexp" '(progn (major-mode) (mino-mode-1 1) (minor-mode-2 1) ...)) Because it's no longer a declarative data structure that can be queried and modified, rather an imperative program. -- Kevin Rodgers Denver, Colorado, USA ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: image-toggle-display overwrites nxml-mode local key map 2009-12-03 3:08 ` Kevin Rodgers @ 2009-12-03 3:31 ` Lennart Borgman 0 siblings, 0 replies; 22+ messages in thread From: Lennart Borgman @ 2009-12-03 3:31 UTC (permalink / raw) To: Kevin Rodgers, 5062; +Cc: bug-gnu-emacs On Thu, Dec 3, 2009 at 4:08 AM, Kevin Rodgers <kevin.d.rodgers@gmail.com> wrote: > Lennart Borgman wrote: >> >> On Thu, Dec 3, 2009 at 1:59 AM, Juri Linkov <juri@jurta.org> wrote: >>>>> >>>>> Or did you mean a joint mode like `c-mode-and-image-minor-mode', >>>>> `nxml-mode-and-image-minor-mode', `ps-mode-and-doc-view-minor-mode'? >>>>> Wouldn't this be too clumsy? >>>> >>>> Yes, why would it be too clumsy? >>>> >>>> A more flexibel way might be to use define-globalized-minor-mode. The >>>> turn on function there could make any check. It could for example look >>>> in a list similar to auto-mode-alist, but for minor modes. >>>> >>>> But maybe that would take too long time? >>> >>> I think Stefan's idea of allowing auto-mode-alist to have entries like >>> ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)) is more universal. >> >> >> Why not allow a form there then: >> >> ("regexp" '(progn (major-mode) (mino-mode-1 1) (minor-mode-2 1) ...)) > > Because it's no longer a declarative data structure that can be queried and > modified, rather an imperative program. Hm, yes. I use to think it is bad to put a (lambda () ...) in a hook, because you may want to remove it later. And actually I do modify this list to in majmodpri.el so you are right. A simple format is better. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: image-toggle-display overwrites nxml-mode local key map 2009-12-03 1:37 ` Lennart Borgman 2009-12-03 3:08 ` Kevin Rodgers @ 2009-12-03 3:30 ` Stefan Monnier 1 sibling, 0 replies; 22+ messages in thread From: Stefan Monnier @ 2009-12-03 3:30 UTC (permalink / raw) To: Lennart Borgman; +Cc: Tassilo Horn, Brent Goodrick, 5062 >>>> Or did you mean a joint mode like `c-mode-and-image-minor-mode', >>>> `nxml-mode-and-image-minor-mode', `ps-mode-and-doc-view-minor-mode'? >>>> Wouldn't this be too clumsy? >>> >>> Yes, why would it be too clumsy? >>> >>> A more flexibel way might be to use define-globalized-minor-mode. The >>> turn on function there could make any check. It could for example look >>> in a list similar to auto-mode-alist, but for minor modes. >>> >>> But maybe that would take too long time? >> >> I think Stefan's idea of allowing auto-mode-alist to have entries like >> ("regexp" (major-mode minor-mode-1 minor-mode-2 ...)) is more universal. > Why not allow a form there then: > ("regexp" '(progn (major-mode) (mino-mode-1 1) (minor-mode-2 1) ...)) Because you can already do it just fine: (defun myfoo () (major-mode) (mino-mode-1 1) (minor-mode-2 1)) [...] ("regexp" . myfoo) -- Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-11-29 18:33 ` Stefan Monnier 2009-11-29 22:00 ` Lennart Borgman @ 2009-12-03 0:57 ` Juri Linkov 2009-12-03 3:28 ` Stefan Monnier 2009-12-03 5:00 ` Jason Rumney 1 sibling, 2 replies; 22+ messages in thread From: Juri Linkov @ 2009-12-03 0:57 UTC (permalink / raw) To: Stefan Monnier; +Cc: Tassilo Horn, Brent Goodrick, 5062 >> 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-mode-map>\\[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))) \f ;;; Support for bookmark.el (declare-function bookmark-make-record-default "bookmark" -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-12-03 0:57 ` bug#5062: 23.1.50; " Juri Linkov @ 2009-12-03 3:28 ` Stefan Monnier 2009-12-03 5:00 ` Jason Rumney 1 sibling, 0 replies; 22+ messages in thread From: Stefan Monnier @ 2009-12-03 3:28 UTC (permalink / raw) To: Juri Linkov; +Cc: Tassilo Horn, Brent Goodrick, 5062 > If this is ok, I'm ready to update docstrings and commit. Looks good, thank you, Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-12-03 0:57 ` bug#5062: 23.1.50; " Juri Linkov 2009-12-03 3:28 ` Stefan Monnier @ 2009-12-03 5:00 ` Jason Rumney 2009-12-04 0:05 ` Juri Linkov 1 sibling, 1 reply; 22+ messages in thread From: Jason Rumney @ 2009-12-03 5:00 UTC (permalink / raw) To: Juri Linkov, 5062; +Cc: Tassilo Horn, Brent Goodrick Juri Linkov wrote: > 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). > It seems like you are changing the behavior incompatibly, so why keep the name? IIRC the original intention for image-mode-maybe was to use a non-image mode from auto-mode-alist if it existed, otherwise use image-mode. It originated when we were detecting images with magic-mode-alist, which was higher priority than auto-mode-alist. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map 2009-12-03 5:00 ` Jason Rumney @ 2009-12-04 0:05 ` Juri Linkov 0 siblings, 0 replies; 22+ messages in thread From: Juri Linkov @ 2009-12-04 0:05 UTC (permalink / raw) To: Jason Rumney; +Cc: Tassilo Horn, Brent Goodrick, 5062 >> 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). > > It seems like you are changing the behavior incompatibly, so why keep the > name? IIRC the original intention for image-mode-maybe was to use > a non-image mode from auto-mode-alist if it existed, otherwise use > image-mode. Right, so the patch below makes `image-mode-maybe' obsolete as an alias of `image-mode'. This makes it 99% backward-compatible except for rare cases when somehow `auto-mode-alist' doesn't contain a non-image mode - in this case Fundamental Mode is used. And a new name for the function that is a combination of `normal-mode' + `image-minor-mode' is now `image-mode-as-text'. This name avoids using the suffix `-mode' because it's not a real mode. A CVS patch with these changes and updated docstrings: Index: lisp/image-mode.el =================================================================== RCS file: /sources/emacs/emacs/lisp/image-mode.el,v retrieving revision 1.59 diff -c -r1.59 image-mode.el *** lisp/image-mode.el 28 Nov 2009 20:45:22 -0000 1.59 --- lisp/image-mode.el 4 Dec 2009 00:04:52 -0000 *************** *** 42,51 **** ;;;###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 "\\.svgz?\\'") 'xml-mode) auto-mode-alist) ! ;;;###autoload (push (cons (purecopy "\\.svgz?\\'") 'image-mode-maybe) auto-mode-alist) ;;; Image mode window-info management. --- 42,51 ---- ;;;###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) auto-mode-alist) ;;;###autoload (push (cons (purecopy "\\.svgz?\\'") 'xml-mode) auto-mode-alist) ! ;;;###autoload (push (cons (purecopy "\\.svgz?\\'") 'image-mode) auto-mode-alist) ;;; Image mode window-info management. *************** *** 286,291 **** --- 286,294 ---- 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,316 **** map) "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) map) ! "Major mode keymap for viewing images as text in Image mode.") (defvar bookmark-make-record-function) --- 309,319 ---- map) "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) map) ! "Minor mode keymap for viewing images as text in Image mode.") (defvar bookmark-make-record-function) *************** *** 320,487 **** You can use \\<image-mode-map>\\[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 (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") ".")))) ;;;###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 :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 ! (defun image-mode-maybe () ! "Set major or minor mode for image files. ! Set Image major mode only when there are no other major modes ! associated with a filename in `auto-mode-alist'. When an image ! filename matches another major mode in `auto-mode-alist' then ! set that major mode and Image minor mode. ! 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)))) (defun image-toggle-display-text () ! "Showing the text of the image file." ! (if (image-get-display-property) ! (image-toggle-display))) (defvar archive-superior-buffer) (defvar tar-superior-buffer) (declare-function image-refresh "image.c" (spec &optional frame)) (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"))))) \f ;;; Support for bookmark.el (declare-function bookmark-make-record-default "bookmark" --- 323,504 ---- You can use \\<image-mode-map>\\[image-toggle-display] to toggle between display as an image and display as text." (interactive) ! (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 the image fails. ! (if (not (image-get-display-property)) ! (error "Invalid image"))) ! ;; 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))) ! ! (setq mode-name (if image-type (format "Image[%s]" image-type) "Image")) (use-local-map image-mode-map) ! ! ;; 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) ! (run-mode-hooks 'image-mode-hook) ! (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-mode-as-text) ! (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. ! It provides the key \\<image-mode-map>\\[image-toggle-display] \ ! to switch back to `image-mode' ! to display an image file as the actual image." ! 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))) ;;;###autoload ! (defun image-mode-as-text () ! "Set a non-image mode as major mode in combination with image minor mode. ! A non-image major mode found from `auto-mode-alist' or Fundamental mode ! displays an image file as text. `image-minor-mode' provides the key ! \\<image-mode-map>\\[image-toggle-display] to switch back to `image-mode' ! to display an image file as the actual image. ! ! You can use `image-mode-as-text' in `auto-mode-alist' when you want ! to display an image file as text inititally. ! See commands `image-mode' and `image-minor-mode' for more information ! on these modes." (interactive) ! ;; image-mode-as-text = 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 image-mode-as-text)) ! 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 image-mode-as-text)) ! elt)) ! magic-fallback-mode-alist)))) ! (normal-mode) ! (set (make-local-variable 'image-mode-previous-major-mode) major-mode))) ! ;; Restore `image-type' after `kill-all-local-variables' in `normal-mode'. ! (setq image-type previous-image-type) ! ;; Enable image minor mode with `C-c C-c'. ! (image-minor-mode 1) ! ;; Show the image file as text. ! (image-toggle-display-text) ! (message "%s" (concat ! (substitute-command-keys ! "Type \\[image-toggle-display] to view the image as ") ! (if (image-get-display-property) ! "text" "an image") ".")))) ! ! (define-obsolete-function-alias 'image-mode-maybe 'image-mode "23.2") (defun image-toggle-display-text () ! "Show the image file as text. ! Remove text properties that display the image." ! (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 () + "Show 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 `image-mode-as-text' showing the text of ! the image file and `image-mode' showing the image as an image." (interactive) (if (image-get-display-property) ! (image-mode-as-text) ! (image-mode))) \f ;;; Support for bookmark.el (declare-function bookmark-make-record-default "bookmark" -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#5062: marked as done (23.1.50; image-toggle-display overwrites nxml-mode local key map) 2009-11-28 0:44 ` bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map Brent Goodrick 2009-11-28 2:25 ` Stefan Monnier @ 2009-12-04 22:00 ` Emacs bug Tracking System 1 sibling, 0 replies; 22+ messages in thread From: Emacs bug Tracking System @ 2009-12-04 22:00 UTC (permalink / raw) To: Juri Linkov [-- Attachment #1: Type: text/plain, Size: 925 bytes --] Your message dated Fri, 04 Dec 2009 23:47:30 +0200 with message-id <878wdixvgt.fsf@mail.jurta.org> and subject line Re: bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map has caused the Emacs bug report #5062, regarding 23.1.50; image-toggle-display overwrites nxml-mode local key map 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 owner@emacsbugs.donarmstrong.com immediately.) -- 5062: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=5062 Emacs Bug Tracking System Contact owner@emacsbugs.donarmstrong.com with problems [-- Attachment #2: Type: message/rfc822, Size: 9742 bytes --] From: Brent Goodrick <bgoodr@gmail.com> To: emacs-pretest-bug@gnu.org Subject: 23.1.50; image-toggle-display overwrites nxml-mode local key map Date: Fri, 27 Nov 2009 16:44:30 -0800 Message-ID: <e38bce640911271644l148dbc9g1fb378e6b4dbc248@mail.gmail.com> Please write in English if possible, because the Emacs maintainers usually do not have translators to read other languages for them. Your bug report will be posted to the emacs-pretest-bug@gnu.org mailing list. Please describe exactly what actions triggered the bug and the precise symptoms of the bug. If you can, give a recipe starting from `emacs -Q': Insure that you have Emacs 23 built on Debian Squeeze Linux with librsvg2 library (or similar OS's and libraries), just to get the image-mode to interact with nxml-mode in its keybindings for a .svg file which is an XML derivative for SVG files. Then proceed as follows: 1. Store the following XML content into a file called /tmp/test.svg --- cut below this line --- <?xml version="1.0" standalone="no"?> <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> <svg width="400px" height="400px" viewBox="0 0 4000 4000" xmlns="http://www.w3.org/2000/svg" version="1.1"> <title>Sample Title</title> <desc>Sample Description</desc> <g transform="translate(200,200)"> <rect x="0" y="0" width="400" height="400" fill="none" stroke="blue" stroke-width="10px"/> <g transform="translate(50,25)"> <text x="0" y="0" fill="white" stroke="none">10px</text> </g> </g> <g transform="translate(200,1000)"> <rect x="0" y="0" width="400" height="400" fill="none" stroke="green" stroke-width="10px"/> <g transform="translate(50,25)"> <text x="0" y="0" fill="white" stroke="none">1px</text> </g> </g> </svg> --- cut above this line --- 2. Run emacs -Q and wait for it to load and map into the display. 3. Type C-x C-f /tmp/test.svg and see that the image of the file is displayed. 4. Type C-c C-c and note that the XML is shown. All correct behavior so far. 5. Type C-h k C-M-n and notice that the key for C-M-n is bound to `forward-list' which is not correct because the .svg file is a xml file, that, by default, should be bound to `nxml-forward-element' by the defvar for nxml-mode-map inside share/emacs/23.1.50/lisp/nxml/nxml-mode.el.gz. Notice also that nxml-mode applies its own local map with this call inside the `nxml-mode' function: (use-local-map nxml-mode-map) But compare that with `image-toggle-display' where it either calls: (use-local-map image-mode-text-map) or (use-local-map image-mode-map) Neither of which respects the nxml-mode's bindings. For validation, you can inject a "trampoline" function on `use-local-map' to show who overwrites the local map, as follows: (progn (defun xx-new-use-local-map (&rest args) (message "xx-new-use-local-map called from:\n") (backtrace) (apply xx-orig-use-local-map args)) (when (not (boundp 'xx-orig-use-local-map)) (setq xx-orig-use-local-map (symbol-function 'use-local-map)) (fset 'use-local-map 'xx-new-use-local-map))) where the "xx-*" functions were arbitrary for this bug report. Once xx-new-use-local-map is injected as given above, then use C-h C-e to see what happens when you first execute C-x C-f /tmp/test.svg, and then see it again when hitting C-c C-c while in the test.svg buffer. You will see that image-mode function obliterates the local map set by the `nxml-mode' function. I don't know what the correct solution here should be, and many questions arise: a. Should image-mode be a minor mode of nxml-mode, or, b. Should `image-mode' stash a copy of the current buffers (current-local-map), and restore it afterwards, but also applying the special binding for C-c C-c, or, c. Should `image-mode' make the other modes local map be the parent of its own local map? I'm thinking (c)? Thanks to whomever added SVG capabilities to Emacs! bgoodr If Emacs crashed, and you have the Emacs process in the gdb debugger, please include the output from the following gdb commands: `bt full' and `xbacktrace'. For information about debugging Emacs, please read the file /home/brentg/install/Linux.x86_64/share/emacs/23.1.50/etc/DEBUG. Emacs did not crash. In GNU Emacs 23.1.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.18.2) of 2009-10-31 on hungover Windowing system distributor `The X.Org Foundation', version 11.0.10605000 configured using `configure '--with-x-toolkit' '--with-xft' '--prefix=/home/brentg/install/Linux.x86_64'' Important settings: value of $LC_ALL: nil value of $LC_COLLATE: nil value of $LC_CTYPE: nil value of $LC_MESSAGES: nil value of $LC_MONETARY: nil value of $LC_NUMERIC: nil value of $LC_TIME: nil value of $LANG: en_US.UTF-8 value of $XMODIFIERS: nil locale-coding-system: utf-8-unix default enable-multibyte-characters: t Major mode: nXML Minor modes in effect: image-minor-mode: t tooltip-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t global-auto-composition-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Recent input: C-x C-f C-v / t m p / C-x h <backspace> C-b C-b C-b C-g C-x C-f C-SPC M-b C-b <backspace> / t m p C-d C-d C-d C-d C-d / t e s t . s v g <return> C-v <help-echo> <help-echo> <help-echo> <down-mouse-2> <mouse-2> C-x C-s C-x C-v <return> C-c C-c C-h k C-M-n M-x r e p o r t - e m <tab> <return> Recent messages: File mode specification error: (error "Cannot determine image type") call-interactively: End of buffer Mark set Saving file /tmp/test.svg... Wrote /tmp/test.svg Using vacuous schema Type C-c C-c to view the image as text. Repeat this command to go back to displaying the image Type C-x 1 to delete the help window. Load-path shadows: None found. Features: (shadow mail-extr message ecomplete rfc822 mml mml-sec password-cache mm-decode mm-bodies mm-encode mailcap mail-parse rfc2231 rfc2047 rfc2045 qp ietf-drums mailabbrev nnheader gnus-util netrc time-date mm-util mail-prsvr gmm-utils wid-edit mailheader canlock sha1 hex-util hashcash mail-utils emacsbug sendmail help-fns help-mode view nxml-uchnm rng-xsd xsd-regexp rng-cmpct regexp-opt rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns easymenu nxml-mode nxml-outln nxml-rap nxml-util nxml-glyph nxml-enc xmltok cl cl-19 jka-compr image-mode tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image fringe lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar mldrag mouse jit-lock font-lock syntax facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev loaddefs button minibuffer faces cus-face text-properties overlay md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote make-network-process dbusbind gtk x-toolkit x multi-tty emacs) [-- Attachment #3: Type: message/rfc822, Size: 2078 bytes --] From: Juri Linkov <juri@jurta.org> To: Stefan Monnier <monnier@iro.umontreal.ca> Cc: 5062-done@emacsbugs.donarmstrong.com, Brent Goodrick <bgoodr@gmail.com> Subject: Re: bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map Date: Fri, 04 Dec 2009 23:47:30 +0200 Message-ID: <878wdixvgt.fsf@mail.jurta.org> >> If this is ok, I'm ready to update docstrings and commit. > > Looks good, thank you, Done. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2009-12-04 22:00 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <878wdixvgt.fsf@mail.jurta.org> 2009-11-28 0:44 ` bug#5062: 23.1.50; image-toggle-display overwrites nxml-mode local key map Brent Goodrick 2009-11-28 2:25 ` Stefan Monnier 2009-11-28 15:26 ` Brent Goodrick 2009-11-28 17:49 ` Juri Linkov 2009-11-28 20:21 ` Stefan Monnier 2009-11-28 22:54 ` Juri Linkov 2009-11-29 15:36 ` Stefan Monnier 2009-11-29 16:03 ` Juri Linkov 2009-11-29 18:33 ` Stefan Monnier 2009-11-29 22:00 ` Lennart Borgman 2009-11-29 22:08 ` Juri Linkov 2009-11-29 23:16 ` Lennart Borgman 2009-12-03 0:59 ` bug#5062: " Juri Linkov 2009-12-03 1:37 ` Lennart Borgman 2009-12-03 3:08 ` Kevin Rodgers 2009-12-03 3:31 ` Lennart Borgman 2009-12-03 3:30 ` Stefan Monnier 2009-12-03 0:57 ` bug#5062: 23.1.50; " Juri Linkov 2009-12-03 3:28 ` Stefan Monnier 2009-12-03 5:00 ` Jason Rumney 2009-12-04 0:05 ` Juri Linkov 2009-12-04 22:00 ` bug#5062: marked as done (23.1.50; image-toggle-display overwrites nxml-mode local key map) 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).