From: Carl Worth <cworth@cworth.org>
To: Jesse Rosenthal <jrosenthal@jhu.edu>, notmuch@notmuchmail.org
Subject: Re: [PATCH] Fix bug, and clean up code duplication,
	in adding or removing tag by region.
In-Reply-To: <871vejxk94.fsf@jhu.edu>
References: <m1636w7au5.fsf@watt.gilman.jhu.edu> <87sk90ragj.fsf@jhu.edu>
	<87aatfq88h.fsf@yoom.home.cworth.org> <877hogaf7v.fsf@jhu.edu>
	<878w8rkzis.fsf@yoom.home.cworth.org> <871vejxk94.fsf@jhu.edu>
Date: Wed, 14 Apr 2010 10:14:46 -0700
Message-ID: <87zl16ez21.fsf@yoom.home.cworth.org>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Tue, 13 Apr 2010 14:47:19 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wr=
ote:
> There was a bug in notmuch-search-{add,remove}-tag-region, which would
> not behave correctly if the region went beyond the last message. Now,
> instead of simply iterating to the last line of the region, these
> functions will iterate to the minimum of the last line of the region
> and the last possible line, i.e.

Thanks, Jesse!

I tested this and it works great.=20

> (- (line-number-at-pos (point-max)) 2)

The only real problem I see with this approach is that it's fragile in
depending on the buffer having exactly 2 lines of non-thread text at the
end. I can easily see myself wanting to remove the "End of Search
Results" line at the end of the buffer. And if I do that, this code will
break, (tag manipulations will miss the last message).

A more robust fix would check for the ability to read a thread
ID. So making a single function such as
notmuch-search-find-last-line-with-thread-id or so would do the trick
here.

> Also clean up code duplication, as per Carl's suggestion, by making
> notmuch-search-{add/remove}-tag-thread a special case of the -region
> commands, where the region in question is between (point) and (point).

A very nice change as well. My internal alarm on "also" in a commit
message fired, so I took advantage of "git add -p" and "git rebase -i"
to split this portion into a separate commit.

All pushed now.

=2DCarl

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iD8DBQFLxfgG6JDdNq8qSWgRAskPAKCmUrMpHQVoF5mremO+cSQWcDAmawCeLsva
ohf/fSkNwWCquB018kjrZqY=
=sXle
-----END PGP SIGNATURE-----
--=-=-=--