Re: [notmuch] pull request [was Re: pull request]

2010-04-05 Thread Carl Worth
On Sun, 04 Apr 2010 09:08:34 +0100, David Edmondson  wrote:
> It's the 'for-cworth' branch of git://github.com/dme/notmuch.git.
> 
> There's also a simple update to tell git to ignore notmuch-shared.

Thanks. This is pushed.

-Carl


pgpRssLrxCuvt.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [notmuch] pull request [was Re: pull request]

2010-04-04 Thread David Edmondson
On Sun, 04 Apr 2010 08:33:00 +0100, David Edmondson  wrote:
> I'll declare common variables and 'exported' functions in
> 'notmuch-lib.el'. It may take a few days (I'm supposed to be on
> holiday).

It was quicker than expected. Using 'declare-function' in a file that is
required didn't have the right effect (it seemed to be ignored), so the
'declare-function' calls are directly in `notmuch-show.el'. I get no
warnings when compiling now.

It's the 'for-cworth' branch of git://github.com/dme/notmuch.git.

There's also a simple update to tell git to ignore notmuch-shared.

dme.
-- 
David Edmondson, http://dme.org
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [notmuch] pull request [was Re: pull request]

2010-04-04 Thread David Edmondson
On Sat, 03 Apr 2010 18:21:57 -0300, David Bremner  wrote:
> The warnings about unknown functions can be eliminated by use the
> declare-function macro; if you have emacs lisp reference manual (it
> required the package emacs23-common-non-dfsg on Debian) then you can
> run:
> 
> ESC ESC : (info "(elisp)Declaring Functions") 
> 
> For variables, the obvious approach is to make a file 'notmuch-vars.el'
> and put the variables we need in several files there.

I'll declare common variables and 'exported' functions in
'notmuch-lib.el'. It may take a few days (I'm supposed to be on
holiday).

Carl: is a single changeset that creates notmuch-lib.el and moves the
show implementation into notmuch-show.el okay?

dme.
-- 
David Edmondson, http://dme.org
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [notmuch] pull request [was Re: pull request]

2010-04-03 Thread David Bremner
On Sat, 03 Apr 2010 12:37:46 -0700, Carl Worth  wrote:
> 
> If I apply this patch as is, then when compiling the notmuch-show.el I
> get the following warnings:
> 
>   In notmuch-show:
>   notmuch-show.el:969:34:Warning: reference to free variable `notmuch-command'
> 
>   In end of data:
>   notmuch-show.el:983:1:Warning: the following functions are not known to be
>   defined: point-invisible-p, mail-header-extract-no-properties,
>   notmuch-select-tag-with-completion, union, intersection, set-difference,
>   notmuch-search-show-thread, mm-display-parts, mm-dissect-buffer,
>   notmuch-save-attachments, notmuch-count-attachments, notmuch-reply,
>   mm-handle-type, mm-display-part, notmuch-fontify-headers
> 

This is my understanding from staring at the code for a few other
packages; any experts feel free to contradict.

The warnings about unknown functions can be eliminated by use the
declare-function macro; if you have emacs lisp reference manual (it
required the package emacs23-common-non-dfsg on Debian) then you can
run:

ESC ESC : (info "(elisp)Declaring Functions") 

For variables, the obvious approach is to make a file 'notmuch-vars.el'
and put the variables we need in several files there.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [notmuch] pull request [was Re: pull request]

2010-04-03 Thread Carl Worth
On Sat, 03 Apr 2010 07:32:44 +0100, David Edmondson  wrote:
> * commit a9590dfb4efc2c05a35948ef4522c362eb788c10
> | Author: David Edmondson 
> | Date:   Thu Apr 1 11:38:30 2010 +0100
> | 
> | Makefile: Add the emacs directory to load-path when compiling

That's a nice one-line summary of the commit that says "what" the patch
does just fine. But the commit is missing the rest of the commit message
that should give the "why".

What's the motivation of the change? Is something perhaps broken without
this? Or is this a preparation for something else that will be coming
along in a future commit?

From looking at the next commit, I assume this is to enable the
"(require 'notmuch-show)" that is being added subsequently. So I've just
noted that.

> * commit 4de9f3f09e998d7312be2a1c08526e59bbf135a9
> | Author: David Edmondson 
> | Date:   Sun Mar 21 09:54:08 2010 +
> | 
> | emacs/Makefile.local: Use makefile mode

I added similar treatment to the other instances of files named
Makefile.local.

> * commit 94893f25d36aaf43487e111fbfba4f7dae808dd2
> | Author: David Edmondson 
> | Date:   Tue Mar 23 07:04:34 2010 +
> | 
> | emacs/notmuch.el: Improve tag highlighting in search mode
> | 
> | Assume that tags never include an opening bracket, and hence improve
> | the regular expression used to highlight them. This avoids false
> | matches where the 'from' address of a thread participant includes an
> | opening bracket.

Thanks. That's a good fix.

The above are all pushed.

> * commit f7ecad654fd8d0274fc75833d92117c8e496bcea
> | Author: David Edmondson 
> | Date:   Thu Apr 1 18:36:21 2010 +0100
> | 
> | emacs: Move notmuch-show functionality to notmuch-show.el
> | 
> | To ease the transition to a JSON based implementation of
> | `notmuch-show', move the current implementation into a separate file.

This is definitely a nice improvement in modularization. But there are
some aspects of doing multiple-file emacs code that I'm unclear on.

If I apply this patch as is, then when compiling the notmuch-show.el I
get the following warnings:

  In notmuch-show:
  notmuch-show.el:969:34:Warning: reference to free variable `notmuch-command'

  In end of data:
  notmuch-show.el:983:1:Warning: the following functions are not known to be
  defined: point-invisible-p, mail-header-extract-no-properties,
  notmuch-select-tag-with-completion, union, intersection, set-difference,
  notmuch-search-show-thread, mm-display-parts, mm-dissect-buffer,
  notmuch-save-attachments, notmuch-count-attachments, notmuch-reply,
  mm-handle-type, mm-display-part, notmuch-fontify-headers

I can eliminate a few of these by copying the various require calls from
notmuch.el to notmuch-show.el, but that still leaves problems for all of
the functionality defined in notmuch.el and referenced in
notmuch-show.el:

  In notmuch-show:
  notmuch-show.el:973:34:Warning: reference to free variable `notmuch-command'

  In end of data:
  notmuch-show.el:987:1:Warning: the following functions are not known to be
  defined: point-invisible-p, notmuch-select-tag-with-completion,
  notmuch-search-show-thread, notmuch-save-attachments,
  notmuch-count-attachments, notmuch-reply, notmuch-fontify-headers

Does anyone know the right way to fix this? I'd like to get the output
clean as I plan to move the compilation of the emacs code from "make
install-emacs" to "make", (made conditional on a check for the presence
of emacs by configure).

So I haven't merged this commit yet.

-Carl


pgpT1BfG3iojP.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch