From: Michal Sojka <sojkam1@fel.cvut.cz>
To: notmuch@notmuchmail.org
Date: Thu, 28 Jan 2010 16:24:00 +0100
User-Agent: KMail/1.12.4 (Linux/2.6.31.11-amd64; KDE/4.3.4; x86_64; ; )
References: <200912141421.52561.lists@informa.tiker.net>
	<87ljfjfsok.fsf@lillypad.riseup.net>
In-Reply-To: <87ljfjfsok.fsf@lillypad.riseup.net>
MIME-Version: 1.0
Content-Type: Text/Plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit
Message-Id: <201001281624.01577.sojkam1@fel.cvut.cz>
Cc: Andreas =?iso-8859-15?q?Kl=F6ckner?= <lists@informa.tiker.net>
Subject: Re: [notmuch] [patch] store folder information
Precedence: list

On Wednesday 27 of January 2010 16:55:55 micah anderson wrote:
> have not seen a reply from you yet. I'm particularly eager to see this
> get accepted upstream, and it sounds like the changes necessary to do so
> are relatively minor.

Hi Micah and others,

I wanted to test this patch, so I rebased it to the current HEAD. I had to do 
it manually since notmuch evolved significantly since the original posting. 
I'll post in followup mails.

> I'm wondering what your plans are for addressing these issues? I've come
> to depend on this functionality, and would love to see it incorporated
> upstream!
> 
> Specifically these were:
> 
> 1. Unrelated whitespace:

Fixed.
 
> 2. An unrelated hunk creeping in:
> 
> On Tue, 15 Dec 2009 13:22:19 -0800, Carl Worth <cworth@cworth.org> wrote:
> > On Mon, 14 Dec 2009 14:21:50 -0500, Andreas Kl=C3=B6ckner
> > <lists@informa.=
> 
> tiker.net> wrote:
> > > @@ -116,6 +116,8 @@ skip_re_in_subject (const char *subject)
> > >  	    s++;
> > >  	if (strncasecmp (s, "re:", 3) =3D=3D 0)
> > >  	    s +=3D 3;
> > > +        else if (strncasecmp (s, "aw:", 3) =3D=3D 0)
> > > +	    s +=3D 3;
> > >  	else
> > >  	    break;
> > >      }

Fixed.

> 3. Redundant trailing directory name traversal:
> > > +    gchar *full_folder_name =3D NULL;
> > > +    gchar *folder_base_name =3D NULL;
> > > +
> > > +    /* Find name of "folder" containing the email. */
> > > +    full_folder_name =3D g_strdup(path);
> > > +    while (1)
> > > +    {
> > > +        folder_base_name =3D g_path_get_basename(full_folder_name);
> >
> > The trailing directory name is available already during the
> > traversal. So you don't need to search it back out again. See the patch
> > in the following message:
> >
> > 	id:87fx8bygi7.fsf@linux.vnet.ibm.com
> >
> > which simply passes the trailing directory name along, (but skipping a
> > name of "cur" or "new" while traversing).
> 
> 4. supporting hierarchical folders (perhaps this is a later improvement
> 
> that does not need to be added before the original patch is accepted?):
> > Beyond that, though, I imagine some people have hierarchical folders as
> > well, so it probably makes sense to store them as well.
> >
> > To do that, it's probably just a matter of calling gen_terms on the
> > complete filename. I haven't tested, but doing that should allow
> > Xapian's phrase searching to do the right thing for something like:
> >
> > 	filename:portion/of/the/path/name

I leave these two points for later since I do not have time for them now. If 
somebody want to do it, let me know.

Cheers
Michal