To: Aaron Ecay <aaronecay@gmail.com>
Subject: Re: [PATCH] emacs: Improved printing support.
In-Reply-To: <m2vcobbaol.fsf@gmail.com>
References: <1325240897-27525-1-git-send-email-dme@dme.org>
	<1325599946-5936-1-git-send-email-dme@dme.org>
	<m2vcobbaol.fsf@gmail.com>
User-Agent: Notmuch/0.11+64~g42e8f66 (http://notmuchmail.org) Emacs/24.0.92.1
	(x86_64-pc-linux-gnu)
From: David Edmondson <dme@dme.org>
Date: Tue, 17 Jan 2012 09:14:07 +0000
Message-ID: <cunk44qr8rk.fsf@hotblack-desiato.hh.sledj.net>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Cc: notmuch <notmuch@notmuchmail.org>
Precedence: list

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Mon, 16 Jan 2012 16:27:06 -0500, Aaron Ecay <aaronecay@gmail.com> wrote:
> > +;;
>=20
> The above is just a stray comment line, right?

I tend to use them as spacers, but, sure.

> > +(defun notmuch-show-with-message-as-text (fn)
> > +  "Apply function `fn' to a text representation of the current
> > +message."
>=20
> I think the docstring should say what agrs get passed to =E2=80=98fn=E2=
=80=99.  Also,
> function arguments should be referred to with capital letters, and the
> first line should be a complete sentence.  I=E2=80=99d recommend =E2=80=
=9CApply FN to
> (...the rest unchanged)=E2=80=9D for brevity in the first line.
>=20
> (For anyone who is curious, these and other conventions are documented
> in section D.6 of the Elisp manual.)

Will fix.
> > +		      (concat "Tags: "
> > +			      (mapconcat '(lambda (tag) tag) tags ", ") "\n")
>=20
> #'identity instead of the lambda

/me slaps head. What was I thinking?

> > +	 (all (buffer-substring (notmuch-show-message-top)
> > +				(notmuch-show-message-bottom)))
> > +
> > +	 (file (make-temp-file "notmuch")))
> > +    (with-temp-file file
> > +      (insert all)
> > +      (indent-rigidly (point-min) (point-max) (- depth))
> > +      ;; Remove the original header.
> > +      (goto-char (point-min))
> > +      (re-search-forward "^$" (point-max) nil)
> > +      (delete-region (point-min) (point))
> > +      (insert header))
> > +    (funcall fn file (notmuch-show-get-message-properties))
> > +    (delete-file file)))
>=20
> Why does this function write to a file?  It seems that, of the print
> methods, two (ps-print and lpr) don=E2=80=99t use the file at all.  One
> (ps-print/evince) creates yet another file, without depending on the
> first.  The other muttprint functions do use the file.  So:
> - This function would be more general if it didn=E2=80=99t handle file-wr=
iting
>   itself, but rather let each consumer of text-ified messages handle
>   that if needed.
> - It would be cleaner if print backends that don=E2=80=99t crucially depe=
nd on
>   the existence of a file didn=E2=80=99t create one at all.  If muttprint=
 can
>   accept a message on stdin (it looks from the source like it can; it
>   gives me some error message about Iconv Perl modules when I try to run
>   it, so I can=E2=80=99t be sure), maybe writing to a file isn=E2=80=99t =
necessary at
>   all.

I'll rework it, thanks for the review!

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iEYEARECAAYFAk8VO98ACgkQaezQq/BJZRZ8IwCghNfw/w0mKjnD8UbSAj7z7t5w
9CQAn1KdZZIqPT59rZ/FNUTVC7+k9lSR
=wzL6
-----END PGP SIGNATURE-----
--=-=-=--