* [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
@ 2022-01-01 9:31 Allen Li
2022-01-01 9:44 ` Allen Li
0 siblings, 1 reply; 8+ messages in thread
From: Allen Li @ 2022-01-01 9:31 UTC (permalink / raw)
To: emacs-orgmode
The docstring for org-todo (C-c C-t) is unclear about the handling of -
prefix compared to -1 prefix.
Here is the relevant part of the docstring:
With ‘C-u’ prefix ARG, force logging the state change and take a
logging note.
With a ‘C-u C-u’ prefix, switch to the next set of TODO keywords (nextset).
Another way to achieve this is ‘S-C-<right>’.
With a ‘C-u C-u C-u’ prefix, circumvent any state blocking.
With numeric prefix arg, switch to the Nth state.
With a numeric prefix arg of 0, inhibit note taking for the change.
With a numeric prefix arg of -1, cancel repeater to allow marking as DONE.
The actual behavior is that - (along with any other negative numeric
prefix excluding -1) switches to the first state and -1 cancels
the repeater.
It seems like the right fix here is to make - behave the same as -1, and
raise a user error for any other negative numeric prefix, since it is
likely not doing whatever the user wanted.
Emacs : GNU Emacs 27.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.27, cairo version 1.17.4)
of 2021-03-26
Package: Org mode version 9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
2022-01-01 9:31 [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)] Allen Li
@ 2022-01-01 9:44 ` Allen Li
2022-02-07 14:11 ` Ihor Radchenko
0 siblings, 1 reply; 8+ messages in thread
From: Allen Li @ 2022-01-01 9:44 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1.1: Type: text/plain, Size: 482 bytes --]
On Sat, Jan 1, 2022 at 9:31 AM Allen Li <darkfeline@felesatra.moe> wrote:
> It seems like the right fix here is to make - behave the same as -1, and
> raise a user error for any other negative numeric prefix, since it is
> likely not doing whatever the user wanted.
>
Attached a small patch fixing this, which I tested manually.
Feel free to remove the negative arg error if it's not desired (although I
don't think there is a use case for, e.g., typing M-- M-2 instead of M-1).
[-- Attachment #1.2: Type: text/html, Size: 782 bytes --]
[-- Attachment #2: 0001-org-Improve-org-todo-handling-of-negative-prefix-arg.patch --]
[-- Type: text/x-patch, Size: 1056 bytes --]
From 16ff89c309b8bd9aa11183cc9620c56ed96e3ff7 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sat, 1 Jan 2022 01:38:35 -0800
Subject: [PATCH] org: Improve org-todo handling of negative prefix args
* lisp/org.el (org-todo): Handle -1 prefix args consistently and error
on other negative args.
---
lisp/org.el | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lisp/org.el b/lisp/org.el
index e2f315a4c..6b48f660e 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -9785,7 +9785,8 @@ When called through ELisp, arg is also interpreted in the following way:
nil cl
(when (org-invisible-p) (org-end-of-subtree nil t))))
(when (equal arg '(16)) (setq arg 'nextset))
- (when (equal arg -1) (org-cancel-repeater) (setq arg nil))
+ (when (equal (prefix-numeric-value arg) -1) (org-cancel-repeater) (setq arg nil))
+ (when (< (prefix-numeric-value arg) -1) (user-error "Prefix argument %d not supported" arg))
(let ((org-blocker-hook org-blocker-hook)
commentp
case-fold-search)
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
2022-01-01 9:44 ` Allen Li
@ 2022-02-07 14:11 ` Ihor Radchenko
2022-06-18 6:10 ` Ihor Radchenko
0 siblings, 1 reply; 8+ messages in thread
From: Ihor Radchenko @ 2022-02-07 14:11 UTC (permalink / raw)
To: Allen Li; +Cc: emacs-orgmode
Allen Li <darkfeline@felesatra.moe> writes:
> On Sat, Jan 1, 2022 at 9:31 AM Allen Li <darkfeline@felesatra.moe> wrote:
>
>> It seems like the right fix here is to make - behave the same as -1, and
>> raise a user error for any other negative numeric prefix, since it is
>> likely not doing whatever the user wanted.
>>
>
> Attached a small patch fixing this, which I tested manually.
The patch looks reasonable. Marking the message as a patch to make it
appear in https://updates.orgmode.org/
Best,
Ihor
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
2022-02-07 14:11 ` Ihor Radchenko
@ 2022-06-18 6:10 ` Ihor Radchenko
2022-06-26 4:30 ` Allen Li
0 siblings, 1 reply; 8+ messages in thread
From: Ihor Radchenko @ 2022-06-18 6:10 UTC (permalink / raw)
To: Allen Li; +Cc: emacs-orgmode
Ihor Radchenko <yantar92@gmail.com> writes:
> Allen Li <darkfeline@felesatra.moe> writes:
>
>> On Sat, Jan 1, 2022 at 9:31 AM Allen Li <darkfeline@felesatra.moe> wrote:
>>
>>> It seems like the right fix here is to make - behave the same as -1, and
>>> raise a user error for any other negative numeric prefix, since it is
>>> likely not doing whatever the user wanted.
>>>
>>
>> Attached a small patch fixing this, which I tested manually.
Applied onto main via bfd63cc4f.
Would you also be interested to write a test checking org-todo ARGS?
Best,
Ihor
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
2022-06-18 6:10 ` Ihor Radchenko
@ 2022-06-26 4:30 ` Allen Li
2022-06-26 4:49 ` Ihor Radchenko
0 siblings, 1 reply; 8+ messages in thread
From: Allen Li @ 2022-06-26 4:30 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]
On Fri, Jun 17, 2022 at 11:09 PM Ihor Radchenko <yantar92@gmail.com> wrote:
> Ihor Radchenko <yantar92@gmail.com> writes:
>
> > Allen Li <darkfeline@felesatra.moe> writes:
> >
> >> On Sat, Jan 1, 2022 at 9:31 AM Allen Li <darkfeline@felesatra.moe>
> wrote:
> >>
> >>> It seems like the right fix here is to make - behave the same as -1,
> and
> >>> raise a user error for any other negative numeric prefix, since it is
> >>> likely not doing whatever the user wanted.
> >>>
> >>
> >> Attached a small patch fixing this, which I tested manually.
>
> Applied onto main via bfd63cc4f.
>
> Would you also be interested to write a test checking org-todo ARGS?
>
Attached
>
> Best,
> Ihor
>
[-- Attachment #1.2: Type: text/html, Size: 1308 bytes --]
[-- Attachment #2: 0001-test-org-Add-test-for-org-todo-prefix-behavior.patch --]
[-- Type: text/x-patch, Size: 1391 bytes --]
From 9255819ee05b9f2bdbe8de1cf0acbd6ae6553152 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sat, 25 Jun 2022 21:27:20 -0700
Subject: [PATCH] test-org: Add test for org-todo prefix behavior
* testing/lisp/test-org.el (test-org/org-todo-prefix): Add test.
---
testing/lisp/test-org.el | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 121f9efd5..304dddc08 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -7753,6 +7753,25 @@ CLOSED: %s
(org-add-log-note))
(buffer-string))))))
+(ert-deftest test-org/org-todo-prefix ()
+ "Test `org-todo' prefix arg behavior."
+ ;; -1 prefix arg should cancel repeater and mark DONE.
+ (should-not
+ (string-prefix-p
+ "* TODO H"
+ (let ((org-todo-keywords '((sequence "TODO" "DONE"))))
+ (org-test-with-temp-text "* TODO H\n<2012-03-29 Thu +2y>"
+ (org-todo -1)
+ (buffer-string)))))
+ ;; - prefix arg should cancel repeater and mark DONE.
+ (should-not
+ (string-prefix-p
+ "* TODO H"
+ (let ((org-todo-keywords '((sequence "TODO" "DONE"))))
+ (org-test-with-temp-text "* TODO H\n<2012-03-29 Thu +2y>"
+ (org-todo '-)
+ (buffer-string))))))
+
\f
;;; Timestamps API
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
2022-06-26 4:30 ` Allen Li
@ 2022-06-26 4:49 ` Ihor Radchenko
2022-06-26 5:41 ` Allen Li
0 siblings, 1 reply; 8+ messages in thread
From: Ihor Radchenko @ 2022-06-26 4:49 UTC (permalink / raw)
To: Allen Li; +Cc: emacs-orgmode
Allen Li <darkfeline@felesatra.moe> writes:
>> Would you also be interested to write a test checking org-todo ARGS?
>>
>
> Attached
Thanks! However, it is unclear what this test is checking for.
> +(ert-deftest test-org/org-todo-prefix ()
> + "Test `org-todo' prefix arg behavior."
> + ;; -1 prefix arg should cancel repeater and mark DONE.
> + (should-not
> + (string-prefix-p
> + "* TODO H"
> + (let ((org-todo-keywords '((sequence "TODO" "DONE"))))
> + (org-test-with-temp-text "* TODO H\n<2012-03-29 Thu +2y>"
> + (org-todo -1)
> + (buffer-string)))))
This test does not check if the task is actually marked DONE and also
does not check if the repeater is canceled.
Best,
Ihor
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
2022-06-26 4:49 ` Ihor Radchenko
@ 2022-06-26 5:41 ` Allen Li
2022-06-26 6:31 ` Ihor Radchenko
0 siblings, 1 reply; 8+ messages in thread
From: Allen Li @ 2022-06-26 5:41 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1.1: Type: text/plain, Size: 891 bytes --]
Attached
On Sat, Jun 25, 2022 at 9:48 PM Ihor Radchenko <yantar92@gmail.com> wrote:
> Allen Li <darkfeline@felesatra.moe> writes:
>
> >> Would you also be interested to write a test checking org-todo ARGS?
> >>
> >
> > Attached
>
> Thanks! However, it is unclear what this test is checking for.
>
> > +(ert-deftest test-org/org-todo-prefix ()
> > + "Test `org-todo' prefix arg behavior."
> > + ;; -1 prefix arg should cancel repeater and mark DONE.
> > + (should-not
> > + (string-prefix-p
> > + "* TODO H"
> > + (let ((org-todo-keywords '((sequence "TODO" "DONE"))))
> > + (org-test-with-temp-text "* TODO H\n<2012-03-29 Thu +2y>"
> > + (org-todo -1)
> > + (buffer-string)))))
>
> This test does not check if the task is actually marked DONE and also
> does not check if the repeater is canceled.
>
> Best,
> Ihor
>
>
[-- Attachment #1.2: Type: text/html, Size: 1409 bytes --]
[-- Attachment #2: 0001-test-org-Add-test-for-org-todo-prefix-behavior.patch --]
[-- Type: text/x-patch, Size: 1351 bytes --]
From 42156ba1e8b3c589394212ef423f99e122544c5f Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sat, 25 Jun 2022 21:27:20 -0700
Subject: [PATCH] test-org: Add test for org-todo prefix behavior
* testing/lisp/test-org.el (test-org/org-todo-prefix): Add test.
---
testing/lisp/test-org.el | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 121f9efd5..d686c0e3b 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -7753,6 +7753,25 @@ CLOSED: %s
(org-add-log-note))
(buffer-string))))))
+(ert-deftest test-org/org-todo-prefix ()
+ "Test `org-todo' prefix arg behavior."
+ ;; -1 prefix arg should cancel repeater and mark DONE.
+ (should
+ (string-match-p
+ "DONE H\\(.*\n\\)*<2012-03-29 Thu \\+0y>"
+ (let ((org-todo-keywords '((sequence "TODO" "DONE"))))
+ (org-test-with-temp-text "* TODO H\n<2012-03-29 Thu +2y>"
+ (org-todo -1)
+ (buffer-string)))))
+ ;; - prefix arg should cancel repeater and mark DONE.
+ (should
+ (string-match-p
+ "DONE H\\(.*\n\\)*<2012-03-29 Thu \\+0y>"
+ (let ((org-todo-keywords '((sequence "TODO" "DONE"))))
+ (org-test-with-temp-text "* TODO H\n<2012-03-29 Thu +2y>"
+ (org-todo '-)
+ (buffer-string))))))
+
\f
;;; Timestamps API
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)]
2022-06-26 5:41 ` Allen Li
@ 2022-06-26 6:31 ` Ihor Radchenko
0 siblings, 0 replies; 8+ messages in thread
From: Ihor Radchenko @ 2022-06-26 6:31 UTC (permalink / raw)
To: Allen Li; +Cc: emacs-orgmode
Allen Li <darkfeline@felesatra.moe> writes:
> Attached
Thanks!
Applied onto main via 711ada5ac.
I changed the patch slightly adding
;; FIXME: Add tests for all other allowed prefix arguments.
line to the test.
Best,
Ihor
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-06-26 6:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-01 9:31 [BUG] org-todo (C-c C-t) bad negative prefix behavior [9.5.2 (9.5.2-gfbff08 @ /home/ionasal/.emacs.d/elpa/org-9.5.2/)] Allen Li
2022-01-01 9:44 ` Allen Li
2022-02-07 14:11 ` Ihor Radchenko
2022-06-18 6:10 ` Ihor Radchenko
2022-06-26 4:30 ` Allen Li
2022-06-26 4:49 ` Ihor Radchenko
2022-06-26 5:41 ` Allen Li
2022-06-26 6:31 ` Ihor Radchenko
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).