[PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
On Wed, 14 Apr 2010 10:14:46 -0700, Carl Worth wrote:
> 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).
I was a bit worried about that myself. I hard-coded the 2 based on the
hard-coding in `notmuch-search-last-thread' (">"), which currently goes
to the bottom and then goes up two lines. But it seems like if there
were a more robust approach, it could be used in both.
> 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.
It occurs to me that the best way to do this would probably be to go to
point-max, and then (forward-line -1) until we hit a thread-id. That way
we wouldn't have to work all the way down long search indexes. I'll try
to code that up for the next release, and then have
notmuch-search-last-thread use it, as well as the region functions.
Best,
Jesse
[PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
On Wed, 14 Apr 2010 12:50:50 -0500, Jesse Rosenthal wrote: > It occurs to me that the best way to do this would probably be to go to > point-max, and then (forward-line -1) until we hit a thread-id. That way > we wouldn't have to work all the way down long search indexes. I'll try > to code that up for the next release, and then have > notmuch-search-last-thread use it, as well as the region functions. This sounds great, just be careful if this command is run before the buffer has completed loading, as you could be in the middle of a search instead of the end. AFAIK, with the "asynchronous" buffer loading, there's no guarantee that point-max is the end of the search until the other thread has exited. Again, my lisp-fu is very poor, but just a concern I see. -Mark
[PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
On Tue, 13 Apr 2010 14:47:19 -0400, Jesse Rosenthal
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:
Re: [PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
On Tue, 13 Apr 2010 14:47:19 -0400, Jesse Rosenthal jrosent...@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
pgpN8cqte0yOx.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
On Wed, 14 Apr 2010 10:14:46 -0700, Carl Worth cwo...@cworth.org wrote: 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). I was a bit worried about that myself. I hard-coded the 2 based on the hard-coding in `notmuch-search-last-thread' (), which currently goes to the bottom and then goes up two lines. But it seems like if there were a more robust approach, it could be used in both. 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. It occurs to me that the best way to do this would probably be to go to point-max, and then (forward-line -1) until we hit a thread-id. That way we wouldn't have to work all the way down long search indexes. I'll try to code that up for the next release, and then have notmuch-search-last-thread use it, as well as the region functions. Best, Jesse ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
On Wed, 14 Apr 2010 12:50:50 -0500, Jesse Rosenthal jrosent...@jhu.edu wrote: It occurs to me that the best way to do this would probably be to go to point-max, and then (forward-line -1) until we hit a thread-id. That way we wouldn't have to work all the way down long search indexes. I'll try to code that up for the next release, and then have notmuch-search-last-thread use it, as well as the region functions. This sounds great, just be careful if this command is run before the buffer has completed loading, as you could be in the middle of a search instead of the end. AFAIK, with the asynchronous buffer loading, there's no guarantee that point-max is the end of the search until the other thread has exited. Again, my lisp-fu is very poor, but just a concern I see. -Mark ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
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.
(- (line-number-at-pos (point-max)) 2)
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).
---
emacs/notmuch.el | 26 ++
1 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 517c53a..be09f42 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -399,10 +399,11 @@ Complete list of currently available key bindings:
(defun notmuch-search-properties-in-region (property beg end)
(save-excursion
(let ((output nil)
- (last-line (line-number-at-pos end)))
+ (last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
(beginning-of-line)
- (while (<= (line-number-at-pos) last-line)
+ (while (<= (line-number-at-pos) (min last-line max-line))
(setq output (cons (get-text-property (point) property) output))
(forward-line 1))
output)))
@@ -497,38 +498,39 @@ and will also appear in a buffer named \"*Notmuch
errors*\"."
(defun notmuch-search-get-tags-region (beg end)
(save-excursion
(let ((output nil)
- (last-line (line-number-at-pos end)))
+ (last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
- (while (<= (line-number-at-pos) last-line)
+ (while (<= (line-number-at-pos) (min last-line max-line))
(setq output (append output (notmuch-search-get-tags)))
(forward-line 1))
output)))
(defun notmuch-search-add-tag-thread (tag)
- (notmuch-call-notmuch-process "tag" (concat "+" tag)
(notmuch-search-find-thread-id))
- (notmuch-search-set-tags (delete-dups (sort (cons tag
(notmuch-search-get-tags)) 'string<
+ (notmuch-search-add-tag-region tag (point) (point)))
(defun notmuch-search-add-tag-region (tag beg end)
(let ((search-id-string (mapconcat 'identity
(notmuch-search-find-thread-id-region beg end) " or ")))
(notmuch-call-notmuch-process "tag" (concat "+" tag) search-id-string)
(save-excursion
- (let ((last-line (line-number-at-pos end)))
+ (let ((last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
- (while (<= (line-number-at-pos) last-line)
+ (while (<= (line-number-at-pos) (min last-line max-line))
(notmuch-search-set-tags (delete-dups (sort (cons tag
(notmuch-search-get-tags)) 'string<)))
(forward-line))
(defun notmuch-search-remove-tag-thread (tag)
- (notmuch-call-notmuch-process "tag" (concat "-" tag)
(notmuch-search-find-thread-id))
- (notmuch-search-set-tags (delete tag (notmuch-search-get-tags
+ (notmuch-search-remove-tag-region tag (point) (point)))
(defun notmuch-search-remove-tag-region (tag beg end)
(let ((search-id-string (mapconcat 'identity
(notmuch-search-find-thread-id-region beg end) " or ")))
(notmuch-call-notmuch-process "tag" (concat "-" tag) search-id-string)
(save-excursion
- (let ((last-line (line-number-at-pos end)))
+ (let ((last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
- (while (<= (line-number-at-pos) last-line)
+ (while (<= (line-number-at-pos) (min last-line max-line))
(notmuch-search-set-tags (delete tag (notmuch-search-get-tags)))
(forward-line))
--
1.6.5.3
[PATCH] Fix bug, and clean up code duplication, in adding or removing tag by region.
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.
(- (line-number-at-pos (point-max)) 2)
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).
---
emacs/notmuch.el | 26 ++
1 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 517c53a..be09f42 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -399,10 +399,11 @@ Complete list of currently available key bindings:
(defun notmuch-search-properties-in-region (property beg end)
(save-excursion
(let ((output nil)
- (last-line (line-number-at-pos end)))
+ (last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
(beginning-of-line)
- (while (= (line-number-at-pos) last-line)
+ (while (= (line-number-at-pos) (min last-line max-line))
(setq output (cons (get-text-property (point) property) output))
(forward-line 1))
output)))
@@ -497,38 +498,39 @@ and will also appear in a buffer named \*Notmuch
errors*\.
(defun notmuch-search-get-tags-region (beg end)
(save-excursion
(let ((output nil)
- (last-line (line-number-at-pos end)))
+ (last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
- (while (= (line-number-at-pos) last-line)
+ (while (= (line-number-at-pos) (min last-line max-line))
(setq output (append output (notmuch-search-get-tags)))
(forward-line 1))
output)))
(defun notmuch-search-add-tag-thread (tag)
- (notmuch-call-notmuch-process tag (concat + tag)
(notmuch-search-find-thread-id))
- (notmuch-search-set-tags (delete-dups (sort (cons tag
(notmuch-search-get-tags)) 'string
+ (notmuch-search-add-tag-region tag (point) (point)))
(defun notmuch-search-add-tag-region (tag beg end)
(let ((search-id-string (mapconcat 'identity
(notmuch-search-find-thread-id-region beg end) or )))
(notmuch-call-notmuch-process tag (concat + tag) search-id-string)
(save-excursion
- (let ((last-line (line-number-at-pos end)))
+ (let ((last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
- (while (= (line-number-at-pos) last-line)
+ (while (= (line-number-at-pos) (min last-line max-line))
(notmuch-search-set-tags (delete-dups (sort (cons tag
(notmuch-search-get-tags)) 'string)))
(forward-line))
(defun notmuch-search-remove-tag-thread (tag)
- (notmuch-call-notmuch-process tag (concat - tag)
(notmuch-search-find-thread-id))
- (notmuch-search-set-tags (delete tag (notmuch-search-get-tags
+ (notmuch-search-remove-tag-region tag (point) (point)))
(defun notmuch-search-remove-tag-region (tag beg end)
(let ((search-id-string (mapconcat 'identity
(notmuch-search-find-thread-id-region beg end) or )))
(notmuch-call-notmuch-process tag (concat - tag) search-id-string)
(save-excursion
- (let ((last-line (line-number-at-pos end)))
+ (let ((last-line (line-number-at-pos end))
+ (max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
- (while (= (line-number-at-pos) last-line)
+ (while (= (line-number-at-pos) (min last-line max-line))
(notmuch-search-set-tags (delete tag (notmuch-search-get-tags)))
(forward-line))
--
1.6.5.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
