From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Tassilo Horn Newsgroups: gmane.emacs.devel Subject: Re: DocView AutoFitting via "doc-view-autofit-mode" Date: Sun, 01 Apr 2012 11:34:02 +0200 Message-ID: <87sjgnbxth.fsf@thinkpad.tsdh.de> References: <4F764EBB.5030102@googlemail.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1333272863 13100 80.91.229.3 (1 Apr 2012 09:34:23 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sun, 1 Apr 2012 09:34:23 +0000 (UTC) Cc: emacs-devel@gnu.org To: Moritz Maxeiner Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Apr 01 11:34:22 2012 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1SEHAr-0005gE-4H for ged-emacs-devel@m.gmane.org; Sun, 01 Apr 2012 11:34:13 +0200 Original-Received: from localhost ([::1]:38917 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEHAq-000757-HH for ged-emacs-devel@m.gmane.org; Sun, 01 Apr 2012 05:34:12 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:44601) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEHAo-000751-6L for emacs-devel@gnu.org; Sun, 01 Apr 2012 05:34:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SEHAm-0006vo-Ac for emacs-devel@gnu.org; Sun, 01 Apr 2012 05:34:09 -0400 Original-Received: from out1-smtp.messagingengine.com ([66.111.4.25]:36098) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEHAm-0006vj-2N for emacs-devel@gnu.org; Sun, 01 Apr 2012 05:34:08 -0400 Original-Received: from compute4.internal (compute4.nyi.mail.srv.osa [10.202.2.44]) by gateway1.nyi.mail.srv.osa (Postfix) with ESMTP id 3DF1C21662 for ; Sun, 1 Apr 2012 05:34:06 -0400 (EDT) Original-Received: from frontend2.nyi.mail.srv.osa ([10.202.2.161]) by compute4.internal (MEProxy); Sun, 01 Apr 2012 05:34:06 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=from:to:cc:subject:references:date :in-reply-to:message-id:mime-version:content-type; s=smtpout; bh=gb8u7K1Fp0b+98X3DLPVAeq0WpU=; b=F0I2SjJFM7o21Sf81gPbKgVEuhcb kxU8shJ7VWWEc/R9gjICD0lxqqFVmKi0dexyvsNxhOqi9wvI7P2KAdb0W2YsmAVi za5iK5QGkI1r/yM3VkMsDzWd8nlqn95+o1itCleGQCBRmCTgONztXGAZBeU2uKAb dvdSReGyACBXhVk= X-Sasl-enc: EVfUM+OyqNyCETm6JpncySyRhc7Ogett9rgGUbZdSfu8 1333272845 Original-Received: from thinkpad.tsdh.de (91-67-11-43-dynip.superkabel.de [91.67.11.43]) by mail.messagingengine.com (Postfix) with ESMTPSA id DBC9E482598; Sun, 1 Apr 2012 05:34:04 -0400 (EDT) In-Reply-To: <4F764EBB.5030102@googlemail.com> (Moritz Maxeiner's message of "Sat, 31 Mar 2012 02:24:27 +0200") User-Agent: Gnus/5.130004 (Ma Gnus v0.4) Emacs/24.0.94 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.111.4.25 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:149263 Archived-At: Moritz Maxeiner 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 " 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