[PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.

Carl Worth cworth at cworth.org
Wed Apr 14 10:14:46 PDT 2010


On Tue, 13 Apr 2010 14:47:19 -0400, Jesse Rosenthal <jrosenthal at jhu.edu> wrote:
> 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. 

> (- (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.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20100414/93bc32ab/attachment.pgp>


More information about the notmuch mailing list