unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* DocView AutoFitting via "doc-view-autofit-mode"
@ 2012-03-31  0:24 Moritz Maxeiner
  2012-04-01  9:34 ` Tassilo Horn
  0 siblings, 1 reply; 12+ messages in thread
From: Moritz Maxeiner @ 2012-03-31  0:24 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 5298 bytes --]

Hi,
   I wrote this for myself, thought it might be useful for others as well
   and believed this to be the correct mailing list. The patch is against
   the bzr version of somewhere in the last couple of hours.
If anything is missing please tell me,

cal

[Description]

This is a minor mode allowing documents viewed with DocView to be automaticly
refitted to the window they are shown in (no need to hit W/H/P every
time the window size changes).

It is implemented by adding a local hook to window-configuration-change-hook
for the buffer the document is in, which in turn either starts or increases
- if already started - a buffer local timer every time the hook is called.
When the timer runs out, it calls doc-view-fit-[width/height/page]-to-window
according to a buffer local variable.

The timer is used, so that when the hook triggers a lot in a short period
of time it doesn't cause too much overhead (Useful for slower computers).

[ChangeLog]

* doc-view.el (doc-view-autofit-timer-start doc-view-autofit-timer-inc)
   (doc-view-autofit-default-fit doc-view-autofit-mode-map)
   (doc-view-autofit-set doc-view-autofit-width)
   (doc-view-autofit-height doc-view-autofit-page)
   (doc-view-autofit-fit doc-view-autofit-mode): New minor mode
   with customs and functions for automatic fitting in DocView buffers.

[Patch]

*** doc-view.el    2012-03-31 01:55:44.046165270 +0200
--- doc-view.el    2012-03-31 02:02:03.710204323 +0200
*************** See the command `doc-view-mode' for more
*** 1557,1562 ****
--- 1557,1654 ----
          (doc-view-goto-page page)))))


+ ;;;; Automatic fitting minor mode
+
+ (defcustom doc-view-autofit-timer-start 1.0
+   "Initial value (seconds) for the timer that delays the fitting when
+ `doc-view-autofit-fit' is called (Which is when a window
+ configuration change occurs and a document needs to be fitted)."
+   :type 'number
+   :group 'doc-view)
+
+ (defcustom doc-view-autofit-timer-inc 0.02
+   "Value to increase (seconds) the timer (see `doc-view-autofit-timer-start')
+ by, if there is another window configuration change occuring, before
+ it runs out."
+   :type 'number
+   :group 'doc-view)
+
+ (defcustom doc-view-autofit-default-fit 'width
+   "The fitting type initially used when mode is enabled.
+ Valid values are: width, height, page."
+   :type 'symbol
+   :group 'doc-view)
+
+ (defvar doc-view-autofit-mode-map
+   (let ((map (make-sparse-keymap)))
+     (define-key map (kbd "C-c W") 'doc-view-autofit-width)
+     (define-key map (kbd "C-c H") 'doc-view-autofit-height)
+     (define-key map (kbd "C-c P") 'doc-view-autofit-page)
+     map)
+   "Keymap used by `doc-view-autofit-mode'.")
+
+ (defun doc-view-autofit-set (type)
+   "Set autofitting to TYPE for current buffer."
+   (when doc-view-autofit-mode
+     (setq doc-view-autofit-type type)
+     (doc-view-autofit-fit)))
+
+ (defun doc-view-autofit-width ()
+   "Set autofitting to width for current buffer."
+   (interactive) (doc-view-autofit-set 'width))
+
+ (defun doc-view-autofit-height ()
+   "Set autofitting to height for current buffer."
+   (interactive) (doc-view-autofit-set 'height))
+
+ (defun doc-view-autofit-page ()
+   "Set autofitting to page for current buffer."
+   (interactive) (doc-view-autofit-set 'page))
+
+ (defun doc-view-autofit-fit ()
+   "Fits the document in the selected window's buffer
+ delayed with a timer, so multiple calls in succession
+ don't cause as much overhead."
+   (lexical-let
+       ((window (selected-window)))
+     (if (equal doc-view-autofit-timer nil)
+         (setq doc-view-autofit-timer
+               (run-with-timer
+                doc-view-autofit-timer-start nil
+                (lambda ()
+                  (if (window-live-p window)
+                      (save-selected-window
+                        (select-window window)
+                        (cancel-timer doc-view-autofit-timer)
+                        (setq doc-view-autofit-timer nil)
+                        (cond
+                         ((equal 'width doc-view-autofit-type)
+                          (doc-view-fit-width-to-window))
+                         ((equal 'height doc-view-autofit-type)
+                          (doc-view-fit-height-to-window))
+                         ((equal 'page doc-view-autofit-type)
+                          (doc-view-fit-page-to-window))))))))
+       (timer-inc-time doc-view-autofit-timer doc-view-autofit-timer-inc))))
+
+ (define-minor-mode doc-view-autofit-mode
+   "Minor mode for automatic (timer based) fitting in DocView."
+   :lighter " AFit" :keymap doc-view-autofit-mode-map :group 'doc-view
+   (when doc-view-autofit-mode
+     (set (make-local-variable 'doc-view-autofit-type)
+          doc-view-autofit-default-fit)
+     (set (make-local-variable 'doc-view-autofit-timer) nil)
+     (add-hook 'window-configuration-change-hook
+               'doc-view-autofit-fit nil t)
+     (doc-view-autofit-fit))
+   (when (not doc-view-autofit-mode)
+     (remove-hook 'window-configuration-change-hook
+                  'doc-view-autofit-fit t)
+     (when doc-view-autofit-timer
+         (cancel-timer doc-view-autofit-timer)
+         (setq doc-view-autofit-timer nil))
+     (setq doc-view-autofit-type nil)))
+
+
   (provide 'doc-view)

   ;; Local Variables:



[-- Attachment #2: doc-view-autofit.patch --]
[-- Type: text/x-patch, Size: 3654 bytes --]

*** doc-view.el	2012-03-31 01:55:44.046165270 +0200
--- doc-view.el	2012-03-31 02:02:03.710204323 +0200
*************** See the command `doc-view-mode' for more
*** 1557,1562 ****
--- 1557,1654 ----
         (doc-view-goto-page page)))))
  
  
+ ;;;; Automatic fitting minor mode
+ 
+ (defcustom doc-view-autofit-timer-start 1.0
+   "Initial value (seconds) for the timer that delays the fitting when
+ `doc-view-autofit-fit' is called (Which is when a window
+ configuration change occurs and a document needs to be fitted)."
+   :type 'number
+   :group 'doc-view)
+ 
+ (defcustom doc-view-autofit-timer-inc 0.02
+   "Value to increase (seconds) the timer (see `doc-view-autofit-timer-start')
+ by, if there is another window configuration change occuring, before
+ it runs out."
+   :type 'number
+   :group 'doc-view)
+ 
+ (defcustom doc-view-autofit-default-fit 'width
+   "The fitting type initially used when mode is enabled.
+ Valid values are: width, height, page."
+   :type 'symbol
+   :group 'doc-view)
+ 
+ (defvar doc-view-autofit-mode-map
+   (let ((map (make-sparse-keymap)))
+ 	(define-key map (kbd "C-c W") 'doc-view-autofit-width)
+ 	(define-key map (kbd "C-c H") 'doc-view-autofit-height)
+ 	(define-key map (kbd "C-c P") 'doc-view-autofit-page)
+ 	map)
+   "Keymap used by `doc-view-autofit-mode'.")
+ 
+ (defun doc-view-autofit-set (type)
+   "Set autofitting to TYPE for current buffer."
+   (when doc-view-autofit-mode
+ 	(setq doc-view-autofit-type type)
+ 	(doc-view-autofit-fit)))
+ 
+ (defun doc-view-autofit-width ()
+   "Set autofitting to width for current buffer."
+   (interactive) (doc-view-autofit-set 'width))
+ 
+ (defun doc-view-autofit-height ()
+   "Set autofitting to height for current buffer."
+   (interactive) (doc-view-autofit-set 'height))
+ 
+ (defun doc-view-autofit-page ()
+   "Set autofitting to page for current buffer."
+   (interactive) (doc-view-autofit-set 'page))
+ 
+ (defun doc-view-autofit-fit ()
+   "Fits the document in the selected window's buffer
+ delayed with a timer, so multiple calls in succession
+ don't cause as much overhead."
+   (lexical-let
+ 	  ((window (selected-window)))
+ 	(if (equal doc-view-autofit-timer nil)
+ 		(setq doc-view-autofit-timer
+ 			  (run-with-timer
+ 			   doc-view-autofit-timer-start nil
+ 			   (lambda ()
+ 				 (if (window-live-p window)
+ 					 (save-selected-window
+ 					   (select-window window)
+ 					   (cancel-timer doc-view-autofit-timer)
+ 					   (setq doc-view-autofit-timer nil)
+ 					   (cond
+ 						((equal 'width doc-view-autofit-type)
+ 						 (doc-view-fit-width-to-window))
+ 						((equal 'height doc-view-autofit-type)
+ 						 (doc-view-fit-height-to-window))
+ 						((equal 'page doc-view-autofit-type)
+ 						 (doc-view-fit-page-to-window))))))))
+ 	  (timer-inc-time doc-view-autofit-timer doc-view-autofit-timer-inc))))
+ 
+ (define-minor-mode doc-view-autofit-mode
+   "Minor mode for automatic (timer based) fitting in DocView."
+   :lighter " AFit" :keymap doc-view-autofit-mode-map :group 'doc-view
+   (when doc-view-autofit-mode
+ 	(set (make-local-variable 'doc-view-autofit-type)
+ 		 doc-view-autofit-default-fit)
+ 	(set (make-local-variable 'doc-view-autofit-timer) nil)
+ 	(add-hook 'window-configuration-change-hook
+ 			  'doc-view-autofit-fit nil t)
+ 	(doc-view-autofit-fit))
+   (when (not doc-view-autofit-mode)
+ 	(remove-hook 'window-configuration-change-hook
+ 				 'doc-view-autofit-fit t)
+ 	(when doc-view-autofit-timer
+ 		(cancel-timer doc-view-autofit-timer)
+ 		(setq doc-view-autofit-timer nil))
+ 	(setq doc-view-autofit-type nil)))
+ 
+ 
  (provide 'doc-view)
  
  ;; Local Variables:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-03-31  0:24 DocView AutoFitting via "doc-view-autofit-mode" Moritz Maxeiner
@ 2012-04-01  9:34 ` Tassilo Horn
  2012-04-02 17:58   ` Moritz Maxeiner
  0 siblings, 1 reply; 12+ messages in thread
From: Tassilo Horn @ 2012-04-01  9:34 UTC (permalink / raw)
  To: Moritz Maxeiner; +Cc: emacs-devel

Moritz Maxeiner <moritzmaxeiner@googlemail.com> writes:

Hi Moritz,

thanks for hacking on doc-view.

Here are some minor suggestions glancing at your code.

> + (defvar doc-view-autofit-mode-map
> +   (let ((map (make-sparse-keymap)))
> +     (define-key map (kbd "C-c W") 'doc-view-autofit-width)
> +     (define-key map (kbd "C-c H") 'doc-view-autofit-height)
> +     (define-key map (kbd "C-c P") 'doc-view-autofit-page)
> +     map)
> +   "Keymap used by `doc-view-autofit-mode'.")

Bindings starting with "C-c <single-char>" are usually reserved to the
user, so you should use some other binding.  Or maybe another idea was
to make the existing fitting functions accept a prefix arg activating
the autofit mode, e.g., `W' fits width once, `C-1 W' enables width
autofitting.

> + (defun doc-view-autofit-fit ()
> +   "Fits the document in the selected window's buffer
> + delayed with a timer, so multiple calls in succession
> + don't cause as much overhead."
> +   (lexical-let
> +       ((window (selected-window)))

I think doc-view doesn't rely on dynamic scoping of non-special
variables, so you could just enable lexical binding for the complete
file and use just `let'.  See (info "(elisp)Using Lexical Binding").

> +     (if (equal doc-view-autofit-timer nil)

(not doc-view-autofit-timer) would be a bit more conventional.  Or use
just `doc-view-autofit-timer' as expression and swap the branches of the
`if'.

> +         (setq doc-view-autofit-timer
> +               (run-with-timer
> +                doc-view-autofit-timer-start nil
> +                (lambda ()
> +                  (if (window-live-p window)
> +                      (save-selected-window
> +                        (select-window window)
> +                        (cancel-timer doc-view-autofit-timer)
> +                        (setq doc-view-autofit-timer nil)
> +                        (cond
> +                         ((equal 'width doc-view-autofit-type)
> +                          (doc-view-fit-width-to-window))
> +                         ((equal 'height doc-view-autofit-type)
> +                          (doc-view-fit-height-to-window))
> +                         ((equal 'page doc-view-autofit-type)
> +                          (doc-view-fit-page-to-window))))))))

Compare symbols with `eq'.  And make that lambda a named function.

> +       (timer-inc-time doc-view-autofit-timer doc-view-autofit-timer-inc))))
> +
> + (define-minor-mode doc-view-autofit-mode
> +   "Minor mode for automatic (timer based) fitting in DocView."
> +   :lighter " AFit" :keymap doc-view-autofit-mode-map :group 'doc-view
> +   (when doc-view-autofit-mode
> +     (set (make-local-variable 'doc-view-autofit-type)
> +          doc-view-autofit-default-fit)
> +     (set (make-local-variable 'doc-view-autofit-timer) nil)
> +     (add-hook 'window-configuration-change-hook
> +               'doc-view-autofit-fit nil t)
> +     (doc-view-autofit-fit))
> +   (when (not doc-view-autofit-mode)
> +     (remove-hook 'window-configuration-change-hook
> +                  'doc-view-autofit-fit t)
> +     (when doc-view-autofit-timer
> +         (cancel-timer doc-view-autofit-timer)
> +         (setq doc-view-autofit-timer nil))
> +     (setq doc-view-autofit-type nil)))

Why 2 whens instead of one if?

I didn't have time to try it out so far.  But it looks like a useful
addition, especially when using ImageMagick as backend where rescaling
doesn't trigger a reconversion.

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-01  9:34 ` Tassilo Horn
@ 2012-04-02 17:58   ` Moritz Maxeiner
  2012-04-02 18:37     ` Drew Adams
                       ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Moritz Maxeiner @ 2012-04-02 17:58 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 15329 bytes --]

Hi,

  so I updated a few things:

> `C-1 W' enables width
> autofitting.

Done, see the added paragraph in the description, in short, either sets the type and if
off turns on autofitting, or if on and type is already sets turn off autofitting, e.g.
`C-1 W' `C-1 W' turns on autofitting and sets it to width and then turns it off again.

> so you could just enable lexical binding for the complete
> file and use just `let'

It is already enabled (bzr trunk), I just didn't see it. Strangely enough, using
`lexical-let' inside a lexical binding enabled file produces tons of "bad lexing"
errors when used with named functions (It doesn't in lexica-binding disabled files).

e.g. this will produce a [bad-lexical-ref] on `M-x eval-buffer RET' `M-x bar RET', but not
when the file header (which enables lexical-binding) is removed. So far reading the
information about lexical-binding in emacs has not let me to an understanding why, but meh.

;;; foo.el --- Foo -*- lexical-binding: t -*-

(defun foo (window)
   (message "%s" window))

(defun bar ()
   (interactive)
   (lexical-let ((foobar (selected-window)))
     (foo foobar)))

Which is why I made the part you comment on here

> make that lambda a named function

a lambda instead of a named function in the first place (With a lambda is works
both with lexical-binding enabled and not, but not with a named function).

Well, it's let now instead of lexical-let since lexical-binding is enabled
for doc-view anyway.


> Compare symbols with `eq'

Done, but any specific reason for that? `equal's doc-string states that symbols must match exactly,
so in the specific event of symbols, don't equal and eq behave the same?

> Why 2 whens instead of one if?

'cause one alternative of the if has to use progn anyway and at the time I had just copy-pasted
the first and altered it to match. Changed to if now.


If I missed anything you suggested or someone has more suggestions to better the minor mode please
do say so.

Bye,
   cal


[Description]

This is a minor mode allowing documents viewed with DocView to be automaticly
refitted to the window they are shown in (no need to hit W/H/P every
time the window size changes).

Mode function is `doc-view-autofit-mode' and doc-view-fit-[width/height/page]-to-window
have been modified for a prefix argument, which if is non-nil and numeric value 1 does
(type refers to the part in brackets):

1. If autofit mode is off, turn it on with this type
2. If autofit is on and already set to this type: Turn autofit off
3. If autofit is on and not set to this type: Set it to this type


It is implemented by adding a local hook to window-configuration-change-hook
for the buffer the document is in, which in turn either starts or increases
- if already started - a buffer local timer every time the hook is called.
When the timer runs out, it calls doc-view-fit-[width/height/page]-to-window
according to a buffer local variable.

The timer is used, so that when the hook triggers a lot in a short period
of time it doesn't cause too much overhead (Useful for slower computers).

[ChangeLog]

* doc-view.el (doc-view-fit-check-prefix doc-view-fit-width-to-window)
   (doc-view-fit-height-to-window doc-view-fit-page-to-window)
   (doc-view-autofit-timer-start doc-view-autofit-timer-inc)
   (doc-view-autofit-default-fit doc-view-autofit-timer-function) doc-view-fit-height-to-window
   (doc-view-autofit-fit doc-view-autofit-mode): New minor mode
   with customs and functions for automatic fitting in DocView buffers;
   also added prefix argument handling to doc-view-fit-*-to-window for
   setting the autofitting type easily.

[Patch]

*** doc-view.el    2012-04-02 18:49:03.380742235 +0200
--- doc-view.el    2012-04-02 19:09:03.163769708 +0200
*************** OpenDocument format)."
*** 676,752 ****
     (interactive (list doc-view-shrink-factor))
     (doc-view-enlarge (/ 1.0 factor)))

! (defun doc-view-fit-width-to-window ()
!   "Fit the image width to the window width."
!   (interactive)
!   (let ((win-width (- (nth 2 (window-inside-pixel-edges))
!                       (nth 0 (window-inside-pixel-edges))))
!         (slice (doc-view-current-slice)))
!     (if (not slice)
!         (let ((img-width (car (image-display-size
!                                (image-get-display-property) t))))
!           (doc-view-enlarge (/ (float win-width) (float img-width))))
!
!       ;; If slice is set
!       (let* ((slice-width (nth 2 slice))
!              (scale-factor (/ (float win-width) (float slice-width)))
!              (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
!
!         (doc-view-enlarge scale-factor)
!         (setf (doc-view-current-slice) new-slice)
!         (doc-view-goto-page (doc-view-current-page))))))
!
! (defun doc-view-fit-height-to-window ()
!   "Fit the image height to the window height."
!   (interactive)
!   (let ((win-height (- (nth 3 (window-inside-pixel-edges))
!                        (nth 1 (window-inside-pixel-edges))))
!         (slice (doc-view-current-slice)))
!     (if (not slice)
!         (let ((img-height (cdr (image-display-size
!                                 (image-get-display-property) t))))
!           ;; When users call 'doc-view-fit-height-to-window',
!           ;; they might want to go to next page by typing SPC
!           ;; ONLY once. So I used '(- win-height 1)' instead of
!           ;; 'win-height'
!           (doc-view-enlarge (/ (float (- win-height 1)) (float img-height))))
!
!       ;; If slice is set
!       (let* ((slice-height (nth 3 slice))
!              (scale-factor (/ (float (- win-height 1)) (float slice-height)))
!              (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
!
!         (doc-view-enlarge scale-factor)
!         (setf (doc-view-current-slice) new-slice)
!         (doc-view-goto-page (doc-view-current-page))))))
!
! (defun doc-view-fit-page-to-window ()
!   "Fit the image to the window.
   More specifically, this function enlarges image by:

   min {(window-width / image-width), (window-height / image-height)} times."
!   (interactive)
!   (let ((win-width (- (nth 2 (window-inside-pixel-edges))
!                       (nth 0 (window-inside-pixel-edges))))
!         (win-height (- (nth 3 (window-inside-pixel-edges))
!                        (nth 1 (window-inside-pixel-edges))))
!         (slice (doc-view-current-slice)))
!     (if (not slice)
!         (let ((img-width (car (image-display-size
!                                (image-get-display-property) t)))
!               (img-height (cdr (image-display-size
!                                 (image-get-display-property) t))))
!           (doc-view-enlarge (min (/ (float win-width) (float img-width))
!                                  (/ (float (- win-height 1)) (float img-height)))))
!       ;; If slice is set
!       (let* ((slice-width (nth 2 slice))
!              (slice-height (nth 3 slice))
!              (scale-factor (min (/ (float win-width) (float slice-width))
!                                 (/ (float (- win-height 1)) (float slice-height))))
!              (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
!         (doc-view-enlarge scale-factor)
!         (setf (doc-view-current-slice) new-slice)
!         (doc-view-goto-page (doc-view-current-page))))))

   (defun doc-view-reconvert-doc ()
     "Reconvert the current document.
--- 676,773 ----
     (interactive (list doc-view-shrink-factor))
     (doc-view-enlarge (/ 1.0 factor)))

! (defun doc-view-fit-check-prefix (arg type)
!   "Called by `doc-view-fit-width-to-window', `doc-view-fit-height-to-window'
! and `doc-view-fit-page-to-window' to determine if a prefix argument
! was used and call the appropriate function for each value.
!
! Numeric value 1: Handle automatic fitting"
!   (if (and arg
!            (= (prefix-numeric-value arg) 1))
!       (if doc-view-autofit-mode
!           (if (eq type doc-view-autofit-type)
!               (doc-view-autofit-mode 'toggle)
!             (setq doc-view-autofit-type type) t)
!         (set (make-local-variable 'doc-view-autofit-type) type)
!         (doc-view-autofit-mode) nil) t))
!
! (defun doc-view-fit-width-to-window (arg)
!   "Fit the image width to the window width (Non-nil prefix argument
! with numeric value 1 enables doing this automaticly)."
!   (interactive "P")
!   (when (doc-view-fit-check-prefix arg 'width)
!     (let ((win-width (- (nth 2 (window-inside-pixel-edges))
!                         (nth 0 (window-inside-pixel-edges))))
!           (slice (doc-view-current-slice)))
!       (if (not slice)
!           (let ((img-width (car (image-display-size
!                                  (image-get-display-property) t))))
!             (doc-view-enlarge (/ (float win-width) (float img-width))))
!
!         ;; If slice is set
!         (let* ((slice-width (nth 2 slice))
!                (scale-factor (/ (float win-width) (float slice-width)))
!                (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
!
!           (doc-view-enlarge scale-factor)
!           (setf (doc-view-current-slice) new-slice)
!           (doc-view-goto-page (doc-view-current-page)))))))
!
! (defun doc-view-fit-height-to-window (arg)
!   "Fit the image height to the window height (Non-nil prefix argument
! with numeric value 1 enables doing this automaticly)."
!   (interactive "P")
!   (when (doc-view-fit-check-prefix arg 'height)
!     (let ((win-height (- (nth 3 (window-inside-pixel-edges))
!                          (nth 1 (window-inside-pixel-edges))))
!           (slice (doc-view-current-slice)))
!       (if (not slice)
!           (let ((img-height (cdr (image-display-size
!                                   (image-get-display-property) t))))
!             ;; When users call 'doc-view-fit-height-to-window',
!             ;; they might want to go to next page by typing SPC
!             ;; ONLY once. So I used '(- win-height 1)' instead of
!             ;; 'win-height'
!             (doc-view-enlarge (/ (float (- win-height 1)) (float img-height))))
!
!         ;; If slice is set
!         (let* ((slice-height (nth 3 slice))
!                (scale-factor (/ (float (- win-height 1)) (float slice-height)))
!                (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
!
!           (doc-view-enlarge scale-factor)
!           (setf (doc-view-current-slice) new-slice)
!           (doc-view-goto-page (doc-view-current-page)))))))
!
! (defun doc-view-fit-page-to-window (arg)
!   "Fit the image to the window (Non-nil prefix argument
! with numeric value 1 enables doing this automaticly).
   More specifically, this function enlarges image by:

   min {(window-width / image-width), (window-height / image-height)} times."
!   (interactive "P")
!   (when (doc-view-fit-check-prefix arg 'page)
!     (let ((win-width (- (nth 2 (window-inside-pixel-edges))
!                         (nth 0 (window-inside-pixel-edges))))
!           (win-height (- (nth 3 (window-inside-pixel-edges))
!                          (nth 1 (window-inside-pixel-edges))))
!           (slice (doc-view-current-slice)))
!       (if (not slice)
!           (let ((img-width (car (image-display-size
!                                  (image-get-display-property) t)))
!                 (img-height (cdr (image-display-size
!                                   (image-get-display-property) t))))
!             (doc-view-enlarge (min (/ (float win-width) (float img-width))
!                                    (/ (float (- win-height 1)) (float img-height)))))
!         ;; If slice is set
!         (let* ((slice-width (nth 2 slice))
!                (slice-height (nth 3 slice))
!                (scale-factor (min (/ (float win-width) (float slice-width))
!                                   (/ (float (- win-height 1)) (float slice-height))))
!                (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
!           (doc-view-enlarge scale-factor)
!           (setf (doc-view-current-slice) new-slice)
!           (doc-view-goto-page (doc-view-current-page)))))))

   (defun doc-view-reconvert-doc ()
     "Reconvert the current document.
*************** See the command `doc-view-mode' for more
*** 1557,1562 ****
--- 1578,1654 ----
          (doc-view-goto-page page)))))


+ ;;;; Automatic fitting minor mode
+
+ (defcustom doc-view-autofit-timer-start 1.0
+   "Initial value (seconds) for the timer that delays the fitting when
+ `doc-view-autofit-fit' is called (Which is when a window
+ configuration change occurs and a document needs to be fitted)."
+   :type 'number
+   :group 'doc-view)
+
+ (defcustom doc-view-autofit-timer-inc 0.02
+   "Value to increase (seconds) the timer (see `doc-view-autofit-timer-start')
+ by, if there is another window configuration change occuring, before
+ it runs out."
+   :type 'number
+   :group 'doc-view)
+
+ (defcustom doc-view-autofit-default-fit 'width
+   "The fitting type initially used when mode is enabled.
+ Valid values are: width, height, page."
+   :type 'symbol
+   :group 'doc-view)
+
+ (defun doc-view-autofit-timer-function (window)
+   "The timer function used in `doc-view-autofit-fit' to be called delayed.
+ It expects `doc-view-autofit-timer' and `doc-view-autofit-type' to be
+ set to valid values."
+   (if (window-live-p window)
+       (save-selected-window
+         (select-window window)
+         (cancel-timer doc-view-autofit-timer)
+         (setq doc-view-autofit-timer nil)
+         (cond
+          ((eq 'width doc-view-autofit-type)
+           (doc-view-fit-width-to-window nil))
+          ((eq 'height doc-view-autofit-type)
+           (doc-view-fit-height-to-window nil))
+          ((eq 'page doc-view-autofit-type)
+           (doc-view-fit-page-to-window nil))))))
+
+ (defun doc-view-autofit-fit ()
+   "Fits the document in the selected window's buffer
+ delayed withdoc-view-autofit a timer, so multiple calls in succession
+ don't cause as much overhead."
+   (let ((window (selected-window)))
+     (if doc-view-autofit-timer
+         (timer-inc-time doc-view-autofit-timer doc-view-autofit-timer-inc)
+       (setq doc-view-autofit-timer
+             (run-with-timer doc-view-autofit-timer-start nil
+                             'doc-view-autofit-timer-function window)))))
+
+ (define-minor-mode doc-view-autofit-mode
+   "Minor mode for automatic (timer based) fitting in DocView."
+   :lighter " AFit" :group 'doc-view
+   (if doc-view-autofit-mode
+       (progn (if (or (not (boundp 'doc-view-autofit-type))
+                      (not doc-view-autofit-type))
+                  (set (make-local-variable 'doc-view-autofit-type)
+                       doc-view-autofit-default-fit))
+              (set (make-local-variable 'doc-view-autofit-timer) nil)
+              (add-hook 'window-configuration-change-hook
+                        'doc-view-autofit-fit nil t)
+              (doc-view-autofit-fit))
+     (remove-hook 'window-configuration-change-hook
+                  'doc-view-autofit-fit t)
+     (when doc-view-autofit-timer
+       (cancel-timer doc-view-autofit-timer)
+       (setq doc-view-autofit-timer nil))
+     (setq doc-view-autofit-type nil))
+   doc-view-autofit-mode)
+
+
   (provide 'doc-view)

   ;; Local Variables:

[-- Attachment #2: doc-view-autofit.patch --]
[-- Type: text/x-patch, Size: 10941 bytes --]

*** doc-view.el	2012-04-02 18:49:03.380742235 +0200
--- doc-view.el	2012-04-02 19:09:03.163769708 +0200
*************** OpenDocument format)."
*** 676,752 ****
    (interactive (list doc-view-shrink-factor))
    (doc-view-enlarge (/ 1.0 factor)))
  
! (defun doc-view-fit-width-to-window ()
!   "Fit the image width to the window width."
!   (interactive)
!   (let ((win-width (- (nth 2 (window-inside-pixel-edges))
!                       (nth 0 (window-inside-pixel-edges))))
!         (slice (doc-view-current-slice)))
!     (if (not slice)
!         (let ((img-width (car (image-display-size
!                                (image-get-display-property) t))))
!           (doc-view-enlarge (/ (float win-width) (float img-width))))
! 
!       ;; If slice is set
!       (let* ((slice-width (nth 2 slice))
!              (scale-factor (/ (float win-width) (float slice-width)))
!              (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
! 
!         (doc-view-enlarge scale-factor)
!         (setf (doc-view-current-slice) new-slice)
!         (doc-view-goto-page (doc-view-current-page))))))
! 
! (defun doc-view-fit-height-to-window ()
!   "Fit the image height to the window height."
!   (interactive)
!   (let ((win-height (- (nth 3 (window-inside-pixel-edges))
!                        (nth 1 (window-inside-pixel-edges))))
!         (slice (doc-view-current-slice)))
!     (if (not slice)
!         (let ((img-height (cdr (image-display-size
!                                 (image-get-display-property) t))))
!           ;; When users call 'doc-view-fit-height-to-window',
!           ;; they might want to go to next page by typing SPC
!           ;; ONLY once. So I used '(- win-height 1)' instead of
!           ;; 'win-height'
!           (doc-view-enlarge (/ (float (- win-height 1)) (float img-height))))
! 
!       ;; If slice is set
!       (let* ((slice-height (nth 3 slice))
!              (scale-factor (/ (float (- win-height 1)) (float slice-height)))
!              (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
! 
!         (doc-view-enlarge scale-factor)
!         (setf (doc-view-current-slice) new-slice)
!         (doc-view-goto-page (doc-view-current-page))))))
! 
! (defun doc-view-fit-page-to-window ()
!   "Fit the image to the window.
  More specifically, this function enlarges image by:
  
  min {(window-width / image-width), (window-height / image-height)} times."
!   (interactive)
!   (let ((win-width (- (nth 2 (window-inside-pixel-edges))
!                       (nth 0 (window-inside-pixel-edges))))
!         (win-height (- (nth 3 (window-inside-pixel-edges))
!                        (nth 1 (window-inside-pixel-edges))))
!         (slice (doc-view-current-slice)))
!     (if (not slice)
!         (let ((img-width (car (image-display-size
!                                (image-get-display-property) t)))
!               (img-height (cdr (image-display-size
!                                 (image-get-display-property) t))))
!           (doc-view-enlarge (min (/ (float win-width) (float img-width))
!                                  (/ (float (- win-height 1)) (float img-height)))))
!       ;; If slice is set
!       (let* ((slice-width (nth 2 slice))
!              (slice-height (nth 3 slice))
!              (scale-factor (min (/ (float win-width) (float slice-width))
!                                 (/ (float (- win-height 1)) (float slice-height))))
!              (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
!         (doc-view-enlarge scale-factor)
!         (setf (doc-view-current-slice) new-slice)
!         (doc-view-goto-page (doc-view-current-page))))))
  
  (defun doc-view-reconvert-doc ()
    "Reconvert the current document.
--- 676,773 ----
    (interactive (list doc-view-shrink-factor))
    (doc-view-enlarge (/ 1.0 factor)))
  
! (defun doc-view-fit-check-prefix (arg type)
!   "Called by `doc-view-fit-width-to-window', `doc-view-fit-height-to-window'
! and `doc-view-fit-page-to-window' to determine if a prefix argument
! was used and call the appropriate function for each value.
! 
! Numeric value 1: Handle automatic fitting"
!   (if (and arg
! 		   (= (prefix-numeric-value arg) 1))
! 	  (if doc-view-autofit-mode
! 		  (if (eq type doc-view-autofit-type)
! 			  (doc-view-autofit-mode 'toggle)
! 			(setq doc-view-autofit-type type) t)
! 		(set (make-local-variable 'doc-view-autofit-type) type)
! 		(doc-view-autofit-mode) nil) t))
! 
! (defun doc-view-fit-width-to-window (arg)
!   "Fit the image width to the window width (Non-nil prefix argument
! with numeric value 1 enables doing this automaticly)."
!   (interactive "P")
!   (when (doc-view-fit-check-prefix arg 'width)
! 	(let ((win-width (- (nth 2 (window-inside-pixel-edges))
! 						(nth 0 (window-inside-pixel-edges))))
! 		  (slice (doc-view-current-slice)))
! 	  (if (not slice)
! 		  (let ((img-width (car (image-display-size
! 								 (image-get-display-property) t))))
! 			(doc-view-enlarge (/ (float win-width) (float img-width))))
! 
! 		;; If slice is set
! 		(let* ((slice-width (nth 2 slice))
! 			   (scale-factor (/ (float win-width) (float slice-width)))
! 			   (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
! 
! 		  (doc-view-enlarge scale-factor)
! 		  (setf (doc-view-current-slice) new-slice)
! 		  (doc-view-goto-page (doc-view-current-page)))))))
! 
! (defun doc-view-fit-height-to-window (arg)
!   "Fit the image height to the window height (Non-nil prefix argument
! with numeric value 1 enables doing this automaticly)."
!   (interactive "P")
!   (when (doc-view-fit-check-prefix arg 'height)
! 	(let ((win-height (- (nth 3 (window-inside-pixel-edges))
! 						 (nth 1 (window-inside-pixel-edges))))
! 		  (slice (doc-view-current-slice)))
! 	  (if (not slice)
! 		  (let ((img-height (cdr (image-display-size
! 								  (image-get-display-property) t))))
! 			;; When users call 'doc-view-fit-height-to-window',
! 			;; they might want to go to next page by typing SPC
! 			;; ONLY once. So I used '(- win-height 1)' instead of
! 			;; 'win-height'
! 			(doc-view-enlarge (/ (float (- win-height 1)) (float img-height))))
! 
! 		;; If slice is set
! 		(let* ((slice-height (nth 3 slice))
! 			   (scale-factor (/ (float (- win-height 1)) (float slice-height)))
! 			   (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
! 
! 		  (doc-view-enlarge scale-factor)
! 		  (setf (doc-view-current-slice) new-slice)
! 		  (doc-view-goto-page (doc-view-current-page)))))))
! 
! (defun doc-view-fit-page-to-window (arg)
!   "Fit the image to the window (Non-nil prefix argument
! with numeric value 1 enables doing this automaticly).
  More specifically, this function enlarges image by:
  
  min {(window-width / image-width), (window-height / image-height)} times."
!   (interactive "P")
!   (when (doc-view-fit-check-prefix arg 'page)
! 	(let ((win-width (- (nth 2 (window-inside-pixel-edges))
! 						(nth 0 (window-inside-pixel-edges))))
! 		  (win-height (- (nth 3 (window-inside-pixel-edges))
! 						 (nth 1 (window-inside-pixel-edges))))
! 		  (slice (doc-view-current-slice)))
! 	  (if (not slice)
! 		  (let ((img-width (car (image-display-size
! 								 (image-get-display-property) t)))
! 				(img-height (cdr (image-display-size
! 								  (image-get-display-property) t))))
! 			(doc-view-enlarge (min (/ (float win-width) (float img-width))
! 								   (/ (float (- win-height 1)) (float img-height)))))
! 		;; If slice is set
! 		(let* ((slice-width (nth 2 slice))
! 			   (slice-height (nth 3 slice))
! 			   (scale-factor (min (/ (float win-width) (float slice-width))
! 								  (/ (float (- win-height 1)) (float slice-height))))
! 			   (new-slice (mapcar (lambda (x) (ceiling (* scale-factor x))) slice)))
! 		  (doc-view-enlarge scale-factor)
! 		  (setf (doc-view-current-slice) new-slice)
! 		  (doc-view-goto-page (doc-view-current-page)))))))
  
  (defun doc-view-reconvert-doc ()
    "Reconvert the current document.
*************** See the command `doc-view-mode' for more
*** 1557,1562 ****
--- 1578,1654 ----
         (doc-view-goto-page page)))))
  
  
+ ;;;; Automatic fitting minor mode
+ 
+ (defcustom doc-view-autofit-timer-start 1.0
+   "Initial value (seconds) for the timer that delays the fitting when
+ `doc-view-autofit-fit' is called (Which is when a window
+ configuration change occurs and a document needs to be fitted)."
+   :type 'number
+   :group 'doc-view)
+ 
+ (defcustom doc-view-autofit-timer-inc 0.02
+   "Value to increase (seconds) the timer (see `doc-view-autofit-timer-start')
+ by, if there is another window configuration change occuring, before
+ it runs out."
+   :type 'number
+   :group 'doc-view)
+ 
+ (defcustom doc-view-autofit-default-fit 'width
+   "The fitting type initially used when mode is enabled.
+ Valid values are: width, height, page."
+   :type 'symbol
+   :group 'doc-view)
+ 
+ (defun doc-view-autofit-timer-function (window)
+   "The timer function used in `doc-view-autofit-fit' to be called delayed.
+ It expects `doc-view-autofit-timer' and `doc-view-autofit-type' to be
+ set to valid values."
+   (if (window-live-p window)
+ 	  (save-selected-window
+ 		(select-window window)
+ 		(cancel-timer doc-view-autofit-timer)
+ 		(setq doc-view-autofit-timer nil)
+ 		(cond
+ 		 ((eq 'width doc-view-autofit-type)
+ 		  (doc-view-fit-width-to-window nil))
+ 		 ((eq 'height doc-view-autofit-type)
+ 		  (doc-view-fit-height-to-window nil))
+ 		 ((eq 'page doc-view-autofit-type)
+ 		  (doc-view-fit-page-to-window nil))))))
+ 
+ (defun doc-view-autofit-fit ()
+   "Fits the document in the selected window's buffer
+ delayed withdoc-view-autofit a timer, so multiple calls in succession
+ don't cause as much overhead."
+   (let ((window (selected-window)))
+ 	(if doc-view-autofit-timer
+ 		(timer-inc-time doc-view-autofit-timer doc-view-autofit-timer-inc)
+ 	  (setq doc-view-autofit-timer
+ 			(run-with-timer doc-view-autofit-timer-start nil
+ 							'doc-view-autofit-timer-function window)))))
+ 
+ (define-minor-mode doc-view-autofit-mode
+   "Minor mode for automatic (timer based) fitting in DocView."
+   :lighter " AFit" :group 'doc-view
+   (if doc-view-autofit-mode
+ 	  (progn (if (or (not (boundp 'doc-view-autofit-type))
+ 					 (not doc-view-autofit-type))
+ 				 (set (make-local-variable 'doc-view-autofit-type)
+ 					  doc-view-autofit-default-fit))
+ 			 (set (make-local-variable 'doc-view-autofit-timer) nil)
+ 			 (add-hook 'window-configuration-change-hook
+ 					   'doc-view-autofit-fit nil t)
+ 			 (doc-view-autofit-fit))
+ 	(remove-hook 'window-configuration-change-hook
+ 				 'doc-view-autofit-fit t)
+ 	(when doc-view-autofit-timer
+ 	  (cancel-timer doc-view-autofit-timer)
+ 	  (setq doc-view-autofit-timer nil))
+ 	(setq doc-view-autofit-type nil))
+   doc-view-autofit-mode)
+ 
+ 
  (provide 'doc-view)
  
  ;; Local Variables:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-02 17:58   ` Moritz Maxeiner
@ 2012-04-02 18:37     ` Drew Adams
  2012-04-02 18:41     ` Tassilo Horn
  2012-04-03 13:34     ` Stefan Monnier
  2 siblings, 0 replies; 12+ messages in thread
From: Drew Adams @ 2012-04-02 18:37 UTC (permalink / raw)
  To: 'Moritz Maxeiner', 'Tassilo Horn'; +Cc: emacs-devel

> > Compare symbols with `eq'
> 
> Done, but any specific reason for that? `equal's doc-string 
> states that symbols must match exactly,
> so in the specific event of symbols, don't equal and eq 
> behave the same?

My take on this minor part of your post:

1. Yes, for a symbol, equal and eq act the same.  (Depending on the
implementation there could be a teeny tiny difference in performance.)

2. I use eq for symbol comparison if either (a) the code is sure that both args
are symbols or (b) I want it to behave as if it were sure (e.g. raise an error
if either is not a symbol).

#2 means that the source code indicates my intention.  If a human reader sees eq
then s?he knows that the code is expecting a symbol and will raise an error if
the expectation is not satisfied.  If s?he sees equal she can expect that (a)
the code might not be sure to receive symbols as both args and (b) the code
wants to return non-nil for any two equal objects.

IOW, my use of equal vs eq for expected symbol args is more about what happens
if either of the args is for some reason _not_ a symbol.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-02 17:58   ` Moritz Maxeiner
  2012-04-02 18:37     ` Drew Adams
@ 2012-04-02 18:41     ` Tassilo Horn
  2012-04-03 13:34     ` Stefan Monnier
  2 siblings, 0 replies; 12+ messages in thread
From: Tassilo Horn @ 2012-04-02 18:41 UTC (permalink / raw)
  To: Moritz Maxeiner; +Cc: emacs-devel

Moritz Maxeiner <moritzmaxeiner@googlemail.com> writes:

Hi Moritz,

>  so I updated a few things:
>
>> `C-1 W' enables width autofitting.
>
> Done, see the added paragraph in the description, in short, either
> sets the type and if off turns on autofitting, or if on and type is
> already sets turn off autofitting, e.g.  `C-1 W' `C-1 W' turns on
> autofitting and sets it to width and then turns it off again.

Great.

>> so you could just enable lexical binding for the complete file and
>> use just `let'
>
> It is already enabled (bzr trunk), I just didn't see it. Strangely
> enough, using `lexical-let' inside a lexical binding enabled file
> produces tons of "bad lexing" errors when used with named functions
> (It doesn't in lexica-binding disabled files).

Well, I don't have a clue, too.  But now with emacs 24 and
`lexical-binding', the use of `lexical-let' is discouraged, anyway.

>> Compare symbols with `eq'
>
> Done, but any specific reason for that? `equal's doc-string states
> that symbols must match exactly, so in the specific event of symbols,
> don't equal and eq behave the same?

Yes, the result is the same.  Everything that's `eq' is always also
`eql', and everything that's `eql' is always also `equal'.

It's more a convention to compare symbols and keywords with `eq'.  Then
the reader of your code can see directly that you compare on identity
and not equality.

> If I missed anything you suggested or someone has more suggestions to
> better the minor mode please do say so.

Sorry, this week my spare time is quite limited.  I'll try to look into
it a bit more on the easter weekend.  But since Emacs is in feature
freeze right now, your contribution has to wait until emacs 24.1 is
released, anyway.

It's not a trivial contribution, so you have to assign the copyright to
the FSF in order to have it included in emacs proper.  How that has to
be done is explained here:

  http://www.gnu.org/prep/maintain/html_node/Copyright-Papers.html

Feel free to send me a private mail, if there's something unclear.

> [Patch]

Please use unified diffs (diff -u) instead of context diffs.  I can read
them much better. ;-)

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-02 17:58   ` Moritz Maxeiner
  2012-04-02 18:37     ` Drew Adams
  2012-04-02 18:41     ` Tassilo Horn
@ 2012-04-03 13:34     ` Stefan Monnier
  2012-04-03 13:38       ` Moritz Maxeiner
  2 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-04-03 13:34 UTC (permalink / raw)
  To: Moritz Maxeiner; +Cc: Tassilo Horn, emacs-devel

> It is already enabled (bzr trunk), I just didn't see it. Strangely
> enough, using `lexical-let' inside a lexical binding enabled file
> produces tons of "bad lexing" errors when used with named functions
> (It doesn't in lexica-binding disabled files).

I don't know what "used with named functions" mean.


        Stefan



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-03 13:34     ` Stefan Monnier
@ 2012-04-03 13:38       ` Moritz Maxeiner
  2012-04-03 14:51         ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: Moritz Maxeiner @ 2012-04-03 13:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Tassilo Horn, emacs-devel

Am 4/3/2012 15:34, schrieb Stefan Monnier:
>> It is already enabled (bzr trunk), I just didn't see it. Strangely
>> enough, using `lexical-let' inside a lexical binding enabled file
>> produces tons of "bad lexing" errors when used with named functions
>> (It doesn't in lexica-binding disabled files).
> I don't know what "used with named functions" mean.
>
>
>          Stefan

In the context of what was talked about

(lambda () (message "foo"))

would be an unnamed function, whereas

(defun foo () (message "foo"))

would be a function named "foo"



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-03 13:38       ` Moritz Maxeiner
@ 2012-04-03 14:51         ` Stefan Monnier
  2012-04-03 15:11           ` Moritz Maxeiner
  2012-04-03 15:18           ` Tassilo Horn
  0 siblings, 2 replies; 12+ messages in thread
From: Stefan Monnier @ 2012-04-03 14:51 UTC (permalink / raw)
  To: Moritz Maxeiner; +Cc: Tassilo Horn, emacs-devel

>>> It is already enabled (bzr trunk), I just didn't see it. Strangely
>>> enough, using `lexical-let' inside a lexical binding enabled file
>>> produces tons of "bad lexing" errors when used with named functions
>>> (It doesn't in lexica-binding disabled files).
>> I don't know what "used with named functions" mean.
> In the context of what was talked about
> (lambda () (message "foo"))
> would be an unnamed function, whereas
> (defun foo () (message "foo"))
> would be a function named "foo"

I know, but that doesn't explain the "used with".
Obviously, named functions are used all over the place with
lexical-binding, so they do work in many circumstances.


        Stefan



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-03 14:51         ` Stefan Monnier
@ 2012-04-03 15:11           ` Moritz Maxeiner
  2012-04-03 16:14             ` Stefan Monnier
  2012-04-03 15:18           ` Tassilo Horn
  1 sibling, 1 reply; 12+ messages in thread
From: Moritz Maxeiner @ 2012-04-03 15:11 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Tassilo Horn, emacs-devel

Hm, I seem to have expressed myself a bit poorly,
so let me clear it up:

> Obviously, named functions are used all over the place with
> lexical-binding, so they do work in many circumstances.

I know that and what I wrote was not meant to contradict it, because
you write about general, I wrote about inside lexical-binding enabled files
where lexical-let and let should be the same (since lexical binding is 
already
enabled).

> using
> `lexical-let' inside a lexical binding enabled file produces tons of 
> "bad lexing"
> errors when used with named functions

To clarify, I wrote nothing about `lexical-let' in general, I wrote that 
using it
inside a lexical binding enabled file (meaning with the 
"lexical-binding: t" header)
together with a named function would produce errors.
You can try it out for yourself with the code I send in the same mail 
you refer to:

> ;;; foo.el --- Foo -*- lexical-binding: t -*-
>
> (defun foo (window)
>   (message "%s" window))
>
> (defun bar ()
>   (interactive)
>   (lexical-let ((foobar (selected-window)))
>     (foo foobar)))

Switch to an empty buffer, paste it and then do `M-x eval-buffer' `M-x bar'
and you'll get a [bad-lexical-ref] error.
Remove the header ( the ;;; foo.el line), do `M-x eval-buffer' `M-x bar'
again and you won't get an error, but a message with the selected window.

If you use a lambda inside the defun of bar instead of foo, it will work 
with and without
lexical-binding enabled. And this difference is what I simply don't 
understand.

Now, since I had no idea why the function `lexical-let' would behave 
differently
(if it didn't there should be either an error with AND without the 
header, or
both should work fine), could not find any clues in info/manual/docstring
and more, why it doesn't work, I wrote that.

Moritz



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-03 14:51         ` Stefan Monnier
  2012-04-03 15:11           ` Moritz Maxeiner
@ 2012-04-03 15:18           ` Tassilo Horn
  1 sibling, 0 replies; 12+ messages in thread
From: Tassilo Horn @ 2012-04-03 15:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Moritz Maxeiner, emacs-devel

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>>> It is already enabled (bzr trunk), I just didn't see it. Strangely
>>>> enough, using `lexical-let' inside a lexical binding enabled file
>>>> produces tons of "bad lexing" errors when used with named functions
>>>> (It doesn't in lexica-binding disabled files).
>>> I don't know what "used with named functions" mean.
>> In the context of what was talked about
>> (lambda () (message "foo"))
>> would be an unnamed function, whereas
>> (defun foo () (message "foo"))
>> would be a function named "foo"
>
> I know, but that doesn't explain the "used with".
> Obviously, named functions are used all over the place with
> lexical-binding, so they do work in many circumstances.

Moritz meant you get strange error messages when using lexical-let in a
lexical-binding enabled file.  This is a test case:

--8<---------------cut here---------------start------------->8---
;; -*- lexical-binding: t -*-

(defun foo (arg)
  arg)

(lexical-let ((x 1))
  (foo x))
--8<---------------cut here---------------end--------------->8---

When you eval the last form, you get a message [bad-lexical-ref].  Well,
that's actually not a bad thing, because if the file uses
lexical-binding there's no reason for using lexical-let.  The message is
just not very clear.

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-03 15:11           ` Moritz Maxeiner
@ 2012-04-03 16:14             ` Stefan Monnier
  2012-04-26  3:19               ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-04-03 16:14 UTC (permalink / raw)
  To: Moritz Maxeiner; +Cc: Tassilo Horn, emacs-devel

> Hm, I seem to have expressed myself a bit poorly,
> so let me clear it up:

>> ;;; foo.el --- Foo -*- lexical-binding: t -*-
>> 
>> (defun foo (window)
>> (message "%s" window))
>> 
>> (defun bar ()
>> (interactive)
>> (lexical-let ((foobar (selected-window)))
>> (foo foobar)))

Ah, thanks, I understand now.  Sorry, for reading your earlier message
without enough care.  So the problem has nothing to do with "named
functions".  You can just use

  (lexical-let ((foobar (selected-window)))
    (message "hello %s" foobar))

in a lexical-binding buffer to reproduce the problem.  Indeed, it seems
that lexical-let doesn't work with lexical-binding.  Not a big deal, but
we should at least try to signal it in a way that's less cryptic.


        Stefan



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: DocView AutoFitting via "doc-view-autofit-mode"
  2012-04-03 16:14             ` Stefan Monnier
@ 2012-04-26  3:19               ` Stefan Monnier
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Monnier @ 2012-04-26  3:19 UTC (permalink / raw)
  To: Moritz Maxeiner; +Cc: Tassilo Horn, emacs-devel

>>>>> "Stefan" == Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Hm, I seem to have expressed myself a bit poorly,
>> so let me clear it up:

>>> ;;; foo.el --- Foo -*- lexical-binding: t -*-
>>> 
>>> (defun foo (window)
>>> (message "%s" window))
>>> 
>>> (defun bar ()
>>> (interactive)
>>> (lexical-let ((foobar (selected-window)))
>>> (foo foobar)))

> Ah, thanks, I understand now.  Sorry, for reading your earlier message
> without enough care.  So the problem has nothing to do with "named
> functions".  You can just use

>   (lexical-let ((foobar (selected-window)))
>     (message "hello %s" foobar))

> in a lexical-binding buffer to reproduce the problem.  Indeed, it seems
> that lexical-let doesn't work with lexical-binding.  Not a big deal, but
> we should at least try to signal it in a way that's less cryptic.

I've just installed the patch below in the trunk, which fixes
the problem.


        Stefan


--- lisp/emacs-lisp/cl-macs.el	2012-01-19 07:21:25 +0000
+++ lisp/emacs-lisp/cl-macs.el	2012-04-26 01:25:25 +0000
@@ -1488,13 +1488,19 @@
 		  (list '(defun . cl-defun-expander))
 		  cl-macro-environment))))
     (if (not (get (car (last cl-closure-vars)) 'used))
-	(list 'let (mapcar (function (lambda (x)
-				       (list (caddr x) (cadr x)))) vars)
-	      (sublis (mapcar (function (lambda (x)
+        ;; Turn (let ((foo (gensym))) (set foo <val>) ...(symbol-value foo)...)
+        ;; into (let ((foo <val>)) ...(symbol-value 'foo)...).
+        ;; This is good because it's more efficient but it only works with
+        ;; dynamic scoping, since with lexical scoping we'd need
+        ;; (let ((foo <val>)) ...foo...).
+	`(progn
+           ,@(mapcar (lambda (x) `(defvar ,(caddr x))) vars)
+           (let ,(mapcar (lambda (x) (list (caddr x) (cadr x))) vars)
+           ,(sublis (mapcar (lambda (x)
 					  (cons (caddr x)
-						(list 'quote (caddr x)))))
+                                    (list 'quote (caddr x))))
 			      vars)
-		      ebody))
+                    ebody)))
       (list 'let (mapcar (function (lambda (x)
 				     (list (caddr x)
 					   (list 'make-symbol




^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-04-26  3:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-31  0:24 DocView AutoFitting via "doc-view-autofit-mode" Moritz Maxeiner
2012-04-01  9:34 ` Tassilo Horn
2012-04-02 17:58   ` Moritz Maxeiner
2012-04-02 18:37     ` Drew Adams
2012-04-02 18:41     ` Tassilo Horn
2012-04-03 13:34     ` Stefan Monnier
2012-04-03 13:38       ` Moritz Maxeiner
2012-04-03 14:51         ` Stefan Monnier
2012-04-03 15:11           ` Moritz Maxeiner
2012-04-03 16:14             ` Stefan Monnier
2012-04-26  3:19               ` Stefan Monnier
2012-04-03 15:18           ` Tassilo Horn

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).