procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix a minor todo-mode regression

Browse source 
* lisp/calendar/todo-mode.el (todo-get-overlay): Wrap in
save-excursion.  This fixes a regression introduced by the fix
for bug#27609, whereby trying to raise the priority of the
first item or lower the priority of the last item, which
should be noops, moves point to the item's start.  Clarify
comment.

* test/lisp/calendar/todo-mode-tests.el
(todo-test-raise-lower-priority): Add test cases for trying to
raise first item and lower last item.
(with-todo-test): Clear abbreviated-home-dir, since we change HOME.
(todo-test-toggle-item-header02): Remove ":expected-result
:failed" and tests of point after todo-next-item, since the
effect when using Todo mode is not reproducible in the test
environment.  Add commentary about this.
This commit is contained in:
Stephen Berman 2017-08-11 11:28:57 +02:00
parent a56e6e7961
commit e3ed43f4ac
2 changed files with 56 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
lisp/calendar/todo-mode.el
View file

@ -5381,20 +5381,21 @@ marked) not done todo items."
(defun todo-get-overlay (val)
"Return the overlay at point whose `todo' property has value VAL."
;; When headers are hidden, the display engine makes item's start
;; inaccessible to commands, so go there here, if necessary, in
;; order to check for prefix and header overlays.
(when (memq val '(prefix header))
(unless (looking-at todo-item-start) (todo-item-start)))
;; Use overlays-in to find prefix overlays and check over two
;; positions to find done separator overlay.
(let ((ovs (overlays-in (point) (1+ (point))))
ov)
(catch 'done
(while ovs
(setq ov (pop ovs))
(when (eq (overlay-get ov 'todo) val)
(throw 'done ov))))))
(save-excursion
;; When headers are hidden, the display engine makes item's start
;; inaccessible to commands, so then we have to go there
;; non-interactively to check for prefix and header overlays.
(when (memq val '(prefix header))
(unless (looking-at todo-item-start) (todo-item-start)))
;; Use overlays-in to find prefix overlays and check over two
;; positions to find done separator overlay.
(let ((ovs (overlays-in (point) (1+ (point))))
ov)
(catch 'done
(while ovs
(setq ov (pop ovs))
(when (eq (overlay-get ov 'todo) val)
(throw 'done ov)))))))
(defun todo-marked-item-p ()
"Non-nil if this item begins with `todo-item-mark'.

50
test/lisp/calendar/todo-mode-tests.el
View file

@ -46,6 +46,9 @@
"Set up an isolated todo-mode test environment."
(declare (debug (body)))
`(let* ((todo-test-home (make-temp-file "todo-test-home-" t))
;; Since we change HOME, clear this to avoid a conflict
;; e.g. if Emacs runs within the user's home directory.
(abbreviated-home-dir nil)
(process-environment (cons (format "HOME=%s" todo-test-home)
process-environment))
(todo-directory todo-test-data-dir)
@ -170,7 +173,7 @@ In particular, all lines of a multiline item should be highlighted."
(goto-char (point-min))
(let ((p1 (point))
(s1 (todo-item-string))
p2 s2 p3)
p2 s2 p3 p4)
;; First item in category.
(should (equal p1 (todo-item-start)))
(todo-next-item)
@ -230,7 +233,22 @@ In particular, all lines of a multiline item should be highlighted."
(should (eq (point) p3))
(todo-lower-item-priority)
;; Lowering item priority on a done item is a noop.
(should (eq (point) p3)))))
(should (eq (point) p3))
;; Case 5: raising first item and lowering last item.
(goto-char (point-min)) ; Now on first item.
;; Changing item priority moves point to todo-item-start, so move
;; it away from there for the test.
(end-of-line)
(setq p4 (point))
(todo-raise-item-priority)
;; Raising priority of first item is a noop.
(should (equal (point) p4))
(goto-char (point-max))
(todo-previous-item) ; Now on last item.
(end-of-line)
(setq p4 (point))
(todo-lower-item-priority)
(should (equal (point) p4)))))
(ert-deftest todo-test-todo-mark-unmark-category () ; bug#27609
"Test behavior of todo-mark-category and todo-unmark-category."
@ -426,9 +444,14 @@ the top done item should be the first done item."
;; Header is shown.
(should-not (todo-get-overlay 'header))))
;; FIXME: This test doesn't show the effect of the display overlay on
;; calling todo-next-item in todo-mode: When using Todo mode, the
;; display engine moves point out of the overlay, but here point does
;; not get moved, even when display-graphic-p.
(ert-deftest todo-test-toggle-item-header02 () ; bug#27609
"Test navigating between items with hidden header."
:expected-result :failed ; FIXME
;; This makes no difference for testing todo-next-item.
;; (skip-unless (display-graphic-p))
(with-todo-test
(todo-test--show 2)
(let* ((start0 (point))
@ -448,17 +471,26 @@ the top done item should be the first done item."
;; Point hasn't changed...
(should (eq (point) start0))
(should (looking-at todo-item-start))
;; FIXME: In the test run this puts point at todo-item-start,
;; i.e. the display overlay doesn't affect this movement, unlike
;; with the command in todo-mode (and using call-interactively
;; here doesn't change this).
(todo-next-item)
(should (eq (point) start2))
(should-not (looking-at todo-item-start))
;; FIXME: This should (and when using todo-mode does) put point
;; at the start of the item's test, not at todo-item-start, like
;; todo-previous-item below. But the following tests fail; why?
;; (N.B.: todo-backward-item, called by todo-previous-item,
;; explicitly moves point forward to where it needs to be because
;; otherwise the display engine moves it backward.)
;; (should (eq (point) start2))
;; (should-not (looking-at todo-item-start))
;; And these pass, though they shouldn't:
(should-not (eq (point) start2))
(should (looking-at todo-item-start))
(todo-previous-item)
;; ...but now it has.
(should (eq (point) start1))
(should-not (looking-at todo-item-start))
;; This fails just like the above.
;; (todo-next-item)
;; (should (eq (point) start2))
;; (should-not (looking-at todo-item-start))
;; This is the status quo but is it desirable?
(todo-toggle-item-header)
(should (eq (point) start1))