From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: joaotavora@gmail.com (=?utf-8?B?Sm/Do28gVMOhdm9yYQ==?=) Newsgroups: gmane.emacs.devel Subject: Re: [mentoring] a darkroom/writeroom mode for Emacs Date: Thu, 11 Dec 2014 11:22:37 +0000 Message-ID: References: <20141203142859.24393.98673@vcs.savannah.gnu.org> <20141203215426.GA15791@thyrsus.com> <87ppbzplcw.fsf@newcastle.ac.uk> <83iohr48kr.fsf@gnu.org> <83388u4bps.fsf@gnu.org> <83y4qm2uz9.fsf@gnu.org> <83vblq2los.fsf@gnu.org> <87vblpjq24.fsf@uwakimon.sk.tsukuba.ac.jp> <83mw7119yz.fsf@gnu.org> <87sigqiaaz.fsf@gmx.us> <878uihhv5q.fsf@gmx.us> <87bnnczcjg.fsf@gmx.us> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1418296987 11910 80.91.229.3 (11 Dec 2014 11:23:07 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 11 Dec 2014 11:23:07 +0000 (UTC) Cc: emacs-devel@gnu.org To: Rasmus Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Dec 11 12:23:00 2014 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 1Xz1pj-0005rj-M4 for ged-emacs-devel@m.gmane.org; Thu, 11 Dec 2014 12:22:59 +0100 Original-Received: from localhost ([::1]:50316 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xz1pi-0008Dl-Va for ged-emacs-devel@m.gmane.org; Thu, 11 Dec 2014 06:22:58 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xz1pZ-0008DQ-Mn for emacs-devel@gnu.org; Thu, 11 Dec 2014 06:22:55 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xz1pT-00026S-LL for emacs-devel@gnu.org; Thu, 11 Dec 2014 06:22:49 -0500 Original-Received: from mail-wi0-x22c.google.com ([2a00:1450:400c:c05::22c]:64517) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xz1pT-00026F-AM for emacs-devel@gnu.org; Thu, 11 Dec 2014 06:22:43 -0500 Original-Received: by mail-wi0-f172.google.com with SMTP id n3so14207558wiv.17 for ; Thu, 11 Dec 2014 03:22:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-type:content-transfer-encoding; bh=9plZPas/7IUvFYaUkKl1GIV3p5l+U6WwsehE/Re85Yk=; b=b2z+umxaVLe9iAp9p7Jy+xaYCffGajwDZAl9ZqYEOhh6aIPMRaI1YxxPO33nLXUA9f S82FPwd1fLzIIKuSLpAcZv/4sB6NPFRFgGwvPN8ezrCT0aa8YhQQOtmypDxwZN6waGei QcpSWlHV8poOz+EF2puus1LXYU7MByP5Czgv6LrgytCZPHCRZKMZXxSWTJsNB7qtvohJ cQGKGjNNfiwTjShRodSdSONw3PDpNmAKbQUUGXM7buhljGsun08QWZaQt4sApu9N76An ZW7eptKNspRp60Zn16U1/6ogP0O5sJ/e3TErDKyoRaZUcLRiEbp4wHK2DLsR0/zfieaX QIiQ== X-Received: by 10.180.72.199 with SMTP id f7mr15176191wiv.53.1418296962352; Thu, 11 Dec 2014 03:22:42 -0800 (PST) Original-Received: from king.lan.yourcompany.com (31.57.37.188.rev.vodafone.pt. [188.37.57.31]) by mx.google.com with ESMTPSA id hn2sm1318475wjc.5.2014.12.11.03.22.40 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Dec 2014 03:22:41 -0800 (PST) In-Reply-To: <87bnnczcjg.fsf@gmx.us> (rasmus@gmx.us's message of "Tue, 09 Dec 2014 23:25:07 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.92 (darwin) X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c05::22c 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:179732 Archived-At: Rasmus writes: >> ;; Copyright (C) 2014 Jo=C3=A3o T=C3=A1vora > There's usually a clause somewhere about whether it's part of Emacs. It's > probably not important. That's what M-x auto-insert gave me. I should assign it to the FSF though. > This paragraph is not clear and potentially tautologous. The opening tautology is necessary to diffentiate it from `darkroom-tentative-mode'. I wonder what else is not clear, but I have rewritten it. > You might want to mention the function you wrote below. Maybe it should > be default if you can sort out the FIXME Yes, I had that in mind. >> Value is effective when `darkroom-mode' is toggled, when >> changing window or by calling `darkroom-set-margins'" >> :type 'float >> :group 'darkroom) > So, there's both a function to calculate margins the possibility of > specifying it manually? What takes priority? You found a bug, that second part of the docstring no longer applies. >> (defun darkroom-guess-margins () >> "Guess suitable margins for `darkroom-margins'. > I think there should be a blank line here. I don't think Emacs uses that particular blank, but OK. >> Collects some statistics about the buffer's line lengths, and >> apply a heuristic to figure out how wide to set the margins. If >> the buffer's paragraphs are mostly filled to `fill-column', >> margins should center it on the window, otherwise, margins of >> 0.15 percent are used." > Why the need for hardcoding? To make time for southpark. Also it's cold, typing hurts and naming variables is hard. I've fixed it. >> ;;; FIXME: broken when darkroom-text-scale-increase is anything but >> ;;; 0, since window-width ignores text scaling. Otherwise, a >> ;;; suitable default to put in `darkroom-margins', I guess. > > You can estimate the realized width rolling over some lines and measure. > Probably there's a more appropriate way of doing it.=20=20 > > (save-excursion (- (progn (end-of-visual-line) (point)) > (progn (beginning-of-visual-line) (point)))) > > Note, for understanding this you might get some insights from studying > `line-move' and `line-move-visual' (I don't know). It helped. I dealt with the FIXME with the ultra-horrible `darkroom--real-window-width'. It fixes `window-width' basically. Seems to work. The above won't work because I need to know about window geometry, not line geometry. Unless there's a very big line. And that's how `darkroom--real-window-width' hacks it. >> (let* ((window-width (window-width)) >> (line-widths (save-excursion > I'm not sure this does what you want. A quick test suggests it will find > the end of the line, not the "visual" end of the line. Yes, but I want to collect actual line widths here.=20 > You do you need to add this to the top of your file? > > (eval-when-compile (require 'cl)) I don't think I need it. I use no compile-time cl-functions. > This seems like overenginering. What about something like this (not > tested, check that it works out with the floats). > (let ((n4 (/ (length n) 4))) > (/ (apply '+ (subseq (sort line-width '>) 0 n4)) n4)) Cleaner, needs cl-subseq.. >> "Perhaps turn on visual-line-mode for a better darkro= om?") >> "\n") > Here you can use concat and format. . . In general this functionally > seems a bit obstructive. "Dont's ask me stuff..." OK removed it and replaced with a message. > >> top-quartile-avg window-width)) >> (visual-line-mode 1)) >> 0.15) >> ((> top-quartile-avg (* 0.9 fill-column)) >> (let ((margin (round (/ (- window-width top-quartile-avg) 2)))) >> (cons margin margin))) >> (t >> 0.15))))) > > I think you can simplify this and you should not hardcode .15. Ideas welcome, I didn't think much about the heuristic, this one worked ok. I fixed the hardcoding. >> (defun darkroom-compute-margins () > docstring, please. What's the *idea* of this function. It's an internal function (what version of code were you looking at). It, well, computes margins. This is a big discusison, but while I very much sympathise with your thoroughness in documentation, docstrings should explain the "what" carefully avoiding the "how", so you can change that later. For internal functions, it might be a good idea to leave them out, rather than risk them getting out of date, or hindering others from redesigning your code. I know this from experience. Anyway, to please my mentor, I added a docstring :-) >> (defun darkroom-float-to-columns (f) >> (ceiling (* (let ((edges (window-edges))) >> (- (nth 2 edges) (nth 0 edges))) >> f))) > > (- (line-end-position) (line-beginning-position)) No. Here I'm concerned with the window geometry, not the current line's geometry. >> (defvar darkroom-buffer-margins nil >> "Buffer-local version of `darkroom-margins' defcustom. >> Set by `darkroom-set-margins'") > > I think these should all be at the top of the file. I prefer nearer the users. Unless the file gets big and demands a particular structure, in which case I try to first separate by functionality, not programming construct. >> (walk-windows #'(lambda (w) >> (when (eq (window-buffer w) (current-buffer)) >> 'all-frames))) > > Can't you use with-current-buffer? The walk-windows seems excessive. Keep in mind I need to affect all windows that display that buffer. There might be several. > You should test this when resizing windows and increasing fonts. . . You're right. I'll do that before I submit. Did you test it? >> (setcdr darkroom-buffer-margins >> (round (* (+ 1 increment) (cdr darkroom-buffer-margins)))) > (setq darkroom-buffer-margins (cons =C2=B7 =C2=B7)) I could, but there's no big benefit and this also make the consness explicit. >> (defun darkroom-decrease-margins (decrement) > docstring You either weren't looking at the latest version, or I didn't push it. >> (let ((map (make-sparse-keymap))) >> (define-key map (kbd "C-M-+") 'darkroom-increase-margins) >> (define-key map (kbd "C-M--") 'darkroom-decrease-margins) >> map)) > > Ideally this should be unnecessary. Maybe it should be called after > changing the font size or whatnot. Well, the user might want to tweak it for some reason. But yes, `darkroom--set-margins' should be called from more hooks. >> (defvar darkroom-saved-mode-line-format nil) >> (defvar darkroom-saved-header-line-format nil) >> (defvar darkroom-saved-margins nil) > > Top of file. And docstring. Docstring done. >> (t >> (setq mode-line-format darkroom-saved-mode-line-format >> header-line-format darkroom-saved-header-line-format) >> (text-scale-decrease darkroom-text-scale-increase) > > You need to actually record the size used before the mode is entered. I > could increase the width after I enter darkroom. I started to implement this, then realized, maybe it needn't be so, because it works by increments. Did you try it? >> (t >> ;; (message "debug: buffer: %s windows: %s darkroom-mode: %s" >> ;; (current-buffer) (count-windows) darkroom-mode) >> ))) > > You don't need the last clause. Also, do you want to test if > (eq major-mode 'darkroom-mode) ? Keep it there for future debugging. I don't understand. `darkroom-mode' is a minor mode. > Some comments follow. I did it quickly (as you may notice), but it still > took 1=C2=BD South Park episodes. Which ones are you watching? Jo=C3=A3o