org-encode-time bug

2022-07-22 Thread Morgan Smith
Hello,

I'm using emacs from commit f258f67 (quite recent) and org from commit
39005dc (quite recent).

I'm using native compilation and PGTK.

I was able to reproduce this with 'emacs -Q'

When trying to update a clocktable I get the following backtrace (with a
little bit removed).

Debugger entered--Lisp error: (invalid-function org-encode-time)
  org-encode-time((0 0 0 21 7 2022 4 t -14400))
  org-matcher-time("<2022-07-21 Thu 00:00>")
  org-clock-get-table-data(...)
  org-dblock-write:clocktable(...)
  org-update-dblock()
  org-ctrl-c-ctrl-c(nil)
  funcall-interactively(org-ctrl-c-ctrl-c nil)
  command-execute(org-ctrl-c-ctrl-c)


When I re-evaluate the defun for `org-matcher-time' everything is happy
again.  I'm not sure what the issue is but I assume it's related to
compilation or something.


Thanks,

Morgan



Re: org-encode-time bug

2022-08-18 Thread Morgan Smith


Ihor Radchenko  writes:

> org-encode-time is defined in org-macs.el in the latest Org, but _not_
> in built-in Org. What you are seeing is most likely caused by "mixed"
> installation of Org when part of Org is loaded from built-in Org
> distribution coming from Emacs.

I was actually seeing https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56746
so my bug is fixed.  Thank you!

> Could you please detail on what you did to load the latest org with
> Emacs -Q? Using purely emacs -Q cannot trigger the error simply because
> org-encode-time is absent in the built-in Org.

I think this has to do with the GNU Guix site lisp file loading
packages.  I was meaning to look into this to continue to debug this but
I guess I don't have to since it's been fixed.

Thank you,

Morgan



[PATCH] Org Habit fix + new feature

2022-10-11 Thread Morgan Smith
Hello!

There are two patches here.  The first one is a simple bug fix that
doesn't really have anything to do with the second patch.  It just
happens to be in the same spot of the code.

The second patch allows a habit to be considered done if time was logged
to it.  Imagine you have an org habit like shaving.  Chances are, if you
spend time doing it, it's done.  I like to set LOGGING to nil for these
kinds of habits since it's redundant to have all those state changes
that tell me exactly what the logbook already tells me.

Let me know what you guys think.  Any discussion about the second patch
though shouldn't stop the first one from being applied.

>From cc16dd6a8c59312a75b8e25669a7e4eb3d9f9ef4 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 11 Oct 2022 11:44:26 -0400
Subject: [PATCH 1/2] lisp/org-habit.el: Use time as a history cutoff point

* lisp/org-habit.el (org-habit-parse-todo): Use time as a cutoff point
instead of using a count.

This allows viewing the full history of habits that are completed
multiple times a day.  Previously we would miss some days and show an
incorrect history
---
 lisp/org-habit.el | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/org-habit.el b/lisp/org-habit.el
index 677b7adb6..fe5f4fe63 100644
--- a/lisp/org-habit.el
+++ b/lisp/org-habit.el
@@ -212,11 +212,11 @@ This list represents a \"habit\" for the rest of this module."
 		   habit-entry scheduled-repeat))
 	(setq deadline (+ scheduled (- dr-days sr-days
   (org-back-to-heading t)
-  (let* ((maxdays (+ org-habit-preceding-days org-habit-following-days))
+  (let* ((firstday (- (org-today) org-habit-preceding-days))
 	 (reversed org-log-states-order-reversed)
 	 (search (if reversed 're-search-forward 're-search-backward))
 	 (limit (if reversed end (point)))
-	 (count 0)
+	 (done nil)
 	 (re (format
 		  "^[ \t]*-[ \t]+\\(?:State \"%s\".*%s%s\\)"
 		  (regexp-opt org-done-keywords)
@@ -235,13 +235,14 @@ This list represents a \"habit\" for the rest of this module."
  ("%u" . ".*?")
  ("%U" . ".*?")
 	(unless reversed (goto-char end))
-	(while (and (< count maxdays) (funcall search re limit t))
+	(while (and (not done) (funcall search re limit t))
 	  (push (time-to-days
 		 (org-time-string-to-time
 		  (or (match-string-no-properties 1)
 		  (match-string-no-properties 2
 		closed-dates)
-	  (setq count (1+ count
+  (when (< (car closed-dates) firstday)
+(setq done t
   (list scheduled sr-days deadline dr-days closed-dates sr-type
 
 (defsubst org-habit-scheduled (habit)
-- 
2.38.0

>From 93929b6722e7171551f8390581593e8ea76f1340 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 11 Oct 2022 11:46:39 -0400
Subject: [PATCH 2/2] lisp/org-habit.el: Allow clocking time to complete a
 habit

* lisp/org-habit.el:
(org-habit-clock-completes-habit): Add
(org-habit-parse-todo): Allow clocking time to complete a habit

* etc/ORG-NEWS: Add entry for `org-habit-clock-completes-habit'

* doc/org-manual.org: Add entry for `org-habit-clock-completes-habit'
---
 doc/org-manual.org |  6 ++
 etc/ORG-NEWS   |  5 +
 lisp/org-habit.el  | 18 ++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 428c79923..a3dcfb3f9 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -4423,6 +4423,12 @@ the way habits are displayed in the agenda.
   value is ~t~.  Pressing {{{kbd(C-u K)}}} in the agenda toggles this
   variable.
 
+- ~org-habit-clock-completes-habit~ ::
+
+  #+vindex: org-habit-clock-completes-habit
+  If non-~nil~, a habit is considered done on days that time has been
+  clocked to it.
+
 Lastly, pressing {{{kbd(K)}}} in the agenda buffer causes habits to
 temporarily be disabled and do not appear at all.  Press {{{kbd(K)}}}
 again to bring them back.  They are also subject to tag filtering, if
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index c18c03725..1a3861f60 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -360,6 +360,11 @@ The new setting, when set to non-nil, makes Org create alarm at the
 event time when the alarm time is set to 0.  The default value is
 nil -- do not create alarms at the event time.
 
+*** New custom setting ~org-habit-clock-completes-habit~
+
+The new setting, when set to non-nil, makes Org consider a habit done
+on days that time has been clocked to it.
+
 ** New functions and changes in function arguments
 *** ~org-fold-show-entry~ does not fold drawers by default anymore
 
diff --git a/lisp/org-habit.el b/lisp/org-habit.el
index fe5f4fe63..66aad06c1 100644
--- a/lisp/org-habit.el
+++ b/lisp/org-habit.el
@@ -61,6 +61,11 @@ Note that consistency graphs will overwrite anything else in the buffer."
   :group 'org-habit

Re: [PATCH] Org Habit fix + new feature

2022-10-11 Thread Morgan Smith


Hello,

Colin Baxter  writes:

> Please do not alter the default behaviour. When writing a paper or a
> book I use and need both logging and state changes, and I would prefer
> not to have to spend time changing my setup.

Don't worry, this shouldn't change the default behavior in the
slightest.

Morgan



Re: [PATCH] Org Habit fix + new feature

2022-10-12 Thread Morgan Smith
Ihor Radchenko  writes:
>
> I am not against such feature. However, using clocking will break an
> assumption that a single log record corresponds to a single habit
> completion. This assumption is implied across org-habit code.  
>
Oh that's a good point.  I'll have to go back through the code and see
if that's an issue.



As for your comments on the first patch, let me explain where the
current logic falls short.  Imagine you complete a habit multiple times
in a day because you're using org wrong (yep this also violates that
previous thing that I gotta look into but let's not worry about that for
now).  The current logic simply looks at the previous '(+
org-habit-preceding-days org-habit-following-days)' log records which by
default would be 28.  First of all, why are we looking into the future
at all?  I don't think the habits graph currently supports looking at it
from the perspective of a different day and I think marking things as
complete in the future is pretty odd.  Second of all, if we use org
wrong, then we will start loosing days at the beginning of the graph if
we have more then 28 log records in our period.

Now my patch calculates the first day of the graph and simply looks at
all log records before that date.  This is more robust if we want to use
org wrong. Also it's more intuitive I think.  In many cases I think it
will also be a performance boost since then we likely won't loop the
full 28 times.  Furthermore, this method would support looking at the
habits graph from the perspective of a different day (which blindly
looping 28 times does not).

This patch does not do a good job at adding support for repetitions.
The graph and logic still works in days, not repetitions.  It simply
makes the current code more robust.

> Also, (org-today) does not consider org-extend-today-until. (see
> org-habit-insert-consistency-graphs).

Thanks for catching that!

>
> This logic will fail for non-default combinations of org-log-into-drawer
> + org-clock-into-drawer + org-log-states-order-reversed.

Well shoot.  That's a bummer.  So why are we using a regex here anyways?
It feels not super robust.  Don't we have an AST we could use instead?
Also even if we do want to use regex, pulling out log records and clock
records seems like a pretty common thing to do that should be in a
core library function right?


Thanks for the review!
I appreciate your feedback


Morgan



[PATCH] Testing: Add tests for 'org-agenda-skip-if'

2023-07-15 Thread Morgan Smith
* testing/lisp/test-org-agenda.el (test-org-agenda/skip-if): New test.
(test-org-agenda/non-scheduled-re-matces): Fix typo by changing name
to 'test-org-agenda/non-scheduled-re-matches'.
---
 testing/lisp/test-org-agenda.el | 62 -
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index 22e3106ec..c4bd4f0a7 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -99,7 +99,7 @@
  (looking-at " *agenda-file:Scheduled: *test agenda"
 (org-test-agenda--kill-all-agendas)))
 
-(ert-deftest test-org-agenda/non-scheduled-re-matces ()
+(ert-deftest test-org-agenda/non-scheduled-re-matches ()
   "Make sure that scheduled-looking elements do not appear in agenda.
 See https://list.orgmode.org/20220101200103.gb29...@itccanarias.org/T/#t.";
   (cl-assert (not org-agenda-sticky) nil "precondition violation")
@@ -269,6 +269,66 @@ See 
https://list.orgmode.org/06d301d83d9e$f8b44340$ea1cc9c0$@tomdavey.com";
   (should (not (invisible-p (1- (search-forward "TODO Foo")))
   (org-toggle-sticky-agenda))
 
+(ert-deftest test-org-agenda/skip-if ()
+  "Test `org-agenda-skip-if'."
+  (dolist (options '((scheduled) (notscheduled)
+ (deadline) (notdeadline)
+ (timestamp) (nottimestamp)
+ (regexp "hello") (notregexp "hello")
+ ;; TODO: Test for specific TODO keywords
+ (todo ("*")) (nottodo ("*"
+(should
+ (equal
+  (if (memq (car options) '(notscheduled notdeadline nottimestamp regexp 
nottodo))
+  8
+nil)
+  (org-test-with-temp-text
+  "* hello"
+(org-agenda-skip-if nil options
+(should
+ (equal
+  (if (memq (car options) '(scheduled notdeadline timestamp regexp 
nottodo))
+  36
+nil)
+  (org-test-with-temp-text
+  "* hello
+SCHEDULED: <2023-07-15 Sat>"
+(org-agenda-skip-if nil options
+(should
+ (equal
+  (if (memq (car options) '(notscheduled deadline timestamp regexp 
nottodo))
+  35
+nil)
+  (org-test-with-temp-text
+  "* hello
+DEADLINE: <2023-07-15 Sat>"
+(org-agenda-skip-if nil options
+(should
+ (equal
+  (if (memq (car options) '(notscheduled notdeadline timestamp regexp 
nottodo))
+  25
+nil)
+  (org-test-with-temp-text
+  "* hello
+<2023-07-15 Sat>"
+(org-agenda-skip-if nil options
+(should
+ (equal
+  (if (memq (car options) '(notscheduled notdeadline nottimestamp 
notregexp nottodo))
+  10
+nil)
+  (org-test-with-temp-text
+  "* goodbye"
+(org-agenda-skip-if nil options
+(should
+ (equal
+  (if (memq (car options) '(notscheduled notdeadline nottimestamp 
notregexp todo))
+  26
+nil)
+  (org-test-with-temp-text
+  "* TODO write better tests"
+(org-agenda-skip-if nil options))
+
 (ert-deftest test-org-agenda/goto-date ()
   "Test `org-agenda-goto-date'."
   (unwind-protect
-- 
2.41.0




[PATCH] Testing: Add tests for 'org-agenda-check-for-timestamp-as-reason-to-ignore-todo-item'

2023-07-17 Thread Morgan Smith
* testing/lisp/test-org-agenda.el
(test-org-agenda/check-for-timestamp-as-reason-to-ignore-todo-item):
New test.
---
 testing/lisp/test-org-agenda.el | 56 +
 1 file changed, 56 insertions(+)

diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index c4bd4f0a7..66f2f5bce 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -329,6 +329,62 @@ DEADLINE: <2023-07-15 Sat>"
   "* TODO write better tests"
 (org-agenda-skip-if nil options))
 
+(ert-deftest test-org-agenda/check-for-timestamp-as-reason-to-ignore-todo-item 
()
+  "Test `org-agenda-check-for-timestamp-as-reason-to-ignore-todo-item'."
+  (let ((org-deadline-warning-days 1)
+(expected-return
+ (lambda (timestamp value)
+   (cl-case timestamp
+ (past (not (not (memq value '(past all -1 -2 near)
+ (yesteryesterday (not (not (memq value '(past all -1 -2 near)
+ (yesterday (not (not (memq value '(past all -1 near)
+ (today (not (not (memq value '(all past 0 near)
+ (tomorrow (not (not (memq value '(future all 1 0 near)
+ (tomorroworrow (not (not (memq value '(future all 2 1 0 far)
+ (future (not (not (memq value '(future all 2 1 0 far
+;; Lexically bind the variables we're changing
+org-agenda-todo-ignore-deadlines
+org-agenda-todo-ignore-scheduled
+org-agenda-todo-ignore-timestamp)
+  (org-test-at-time "2023-01-15"
+(dolist (variable '(org-agenda-todo-ignore-deadlines
+org-agenda-todo-ignore-scheduled
+org-agenda-todo-ignore-timestamp))
+  (dolist (type '(timestamp scheduled deadline))
+;; nil is last so it resets the variable for the next one
+(dolist (value `(past future all 2 1 0 -1 -2
+  ,@(when (eq type 'deadline) '(near far 
nil
+  (dolist (timestamp '((past . "<2022-01-15>")
+   (yesteryesterday . "<2023-01-13>")
+   (yesterday . "<2023-01-14>")
+   (today . "<2023-01-15>")
+   (tomorrow . "<2023-01-16>")
+   (tomorroworrow . "<2023-01-17>")
+   (future . "<2024-01-15>")))
+;; Uncomment to debug failure
+;; (message "Type: %S, Variable: %S, Value: %S, Time: %S" type 
variable value (car timestamp))
+(set variable value)
+(should
+ (equal
+  (cl-case variable
+(org-agenda-todo-ignore-deadlines
+ (when (eq type 'deadline)
+   (funcall expected-return (car timestamp) value)))
+(org-agenda-todo-ignore-scheduled
+ (when (eq type 'scheduled)
+   (funcall expected-return (car timestamp) value)))
+(org-agenda-todo-ignore-timestamp
+ (when (eq type 'timestamp)
+   (funcall expected-return (car timestamp) value
+  (org-test-with-temp-text
+  (cl-case type
+(timestamp (concat "* hello " (cdr timestamp)))
+(scheduled (concat "* hello
+SCHEDULED: " (cdr timestamp)))
+(deadline (concat "* hello
+DEADLINE: " (cdr timestamp
+
(org-agenda-check-for-timestamp-as-reason-to-ignore-todo-item)))
+
 (ert-deftest test-org-agenda/goto-date ()
   "Test `org-agenda-goto-date'."
   (unwind-protect
-- 
2.41.0




[PATCH] Testing: Ensure 'org-id-locations-file' is set before updating

2023-07-18 Thread Morgan Smith
Previously, when trying to run the tests in a container limited to the
org repository, it fails because it can't create the directory
"~/.emacs.d/.org-id-locations".

* testing/org-test.el (org-test-load): Move setting
'org-id-locations-file' from here ...
org-test-update-id-locations: ... to here
---

Hello!  This fix allows me to run the tests without giving it access to the
filesystem outside of the repository.  I have no clue what org-id-locations are
and I'm hoping someone else does so I don't have to learn.  I'm not sure if
this is the best fix, but it works.

 testing/org-test.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testing/org-test.el b/testing/org-test.el
index 47687b9f7..3f086f30c 100644
--- a/testing/org-test.el
+++ b/testing/org-test.el
@@ -387,8 +387,6 @@ Tramp related features.  We mostly follow
 (defun org-test-load ()
   "Load up the Org test suite."
   (interactive)
-  (setq org-id-locations-file
-(expand-file-name ".test-org-id-locations" org-test-dir))
   (cl-flet ((rld (base)
 ;; Recursively load all files, if files throw errors
 ;; then silently ignore the error and continue to the
@@ -447,6 +445,8 @@ Tramp related features.  We mostly follow
   (when (buffer-live-p b) (kill-buffer b)
 
 (defun org-test-update-id-locations ()
+  (setq org-id-locations-file
+(expand-file-name ".test-org-id-locations" org-test-dir))
   (org-id-update-id-locations
(directory-files
 org-test-example-dir 'full
-- 
2.41.0




[PATCH] org-clock-sum: Rewrite function to improve performance

2023-07-19 Thread Morgan Smith
* lisp/org-clock.el(org-clock-sum): Rewrite function using
'org-element-map' to traverse the file instead of searching.
---

Hello!

I have a very big file with lots of clock entries and refreshing my clocktable
has become slow.  Using '(benchmark-elapse (org-ctrl-c-ctrl-c))' I saw that it
took 5.660532903 seconds to refresh it!  After this rewrite it only takes
3.384914703 seconds.  Not great, but better.

Thanks,

Morgan

 lisp/org-clock.el | 148 +-
 1 file changed, 54 insertions(+), 94 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 264774032..148af864b 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -33,15 +33,10 @@
 
 (require 'cl-lib)
 (require 'org)
+(require 'org-element)
 
 (declare-function calendar-iso-to-absolute "cal-iso" (date))
 (declare-function notifications-notify "notifications" (&rest params))
-(declare-function org-element-property "org-element-ast" (property node))
-(declare-function org-element-contents-end "org-element" (node))
-(declare-function org-element-end "org-element" (node))
-(declare-function org-element-type "org-element-ast" (node &optional 
anonymous))
-(declare-function org-element-type-p "org-element-ast" (node types))
-(defvar org-element-use-cache)
 (declare-function org-inlinetask-at-task-p "org-inlinetask" ())
 (declare-function org-inlinetask-goto-beginning "org-inlinetask" ())
 (declare-function org-inlinetask-goto-end "org-inlinetask" ())
@@ -1948,100 +1943,65 @@ each headline in the time range with point at the 
headline.  Headlines for
 which HEADLINE-FILTER returns nil are excluded from the clock summation.
 PROPNAME lets you set a custom text property instead of :org-clock-minutes."
   (with-silent-modifications
-(let* ((re (concat "^\\(\\*+\\)[ \t]\\|^[ \t]*"
-  org-clock-string
-  "[ \t]*\\(?:\\(\\[.*?\\]\\)-+\\(\\[.*?\\]\\)\\|=>[ 
\t]+\\([0-9]+\\):\\([0-9]+\\)\\)"))
-  (lmax 30)
-  (ltimes (make-vector lmax 0))
-  (level 0)
-  (tstart (cond ((stringp tstart) (org-time-string-to-seconds tstart))
+(let* ((tstart (cond ((stringp tstart) (org-time-string-to-seconds tstart))
 ((consp tstart) (float-time tstart))
 (t tstart)))
   (tend (cond ((stringp tend) (org-time-string-to-seconds tend))
   ((consp tend) (float-time tend))
   (t tend)))
-  (t1 0)
-  time)
+   (file-total 0))
   (remove-text-properties (point-min) (point-max)
- `(,(or propname :org-clock-minutes) t
-   :org-clock-force-headline-inclusion t))
-  (save-excursion
-   (goto-char (point-max))
-   (while (re-search-backward re nil t)
-  (let* ((element (save-match-data (org-element-at-point)))
- (element-type (org-element-type element)))
-   (cond
-((and (eq element-type 'clock) (match-end 2))
- ;; Two time stamps.
- (let* ((timestamp (org-element-property :value element))
-(ts (float-time
-  (org-encode-time
-   (list 0
- (org-element-property :minute-start timestamp)
- (org-element-property :hour-start timestamp)
- (org-element-property :day-start timestamp)
- (org-element-property :month-start timestamp)
- (org-element-property :year-start timestamp)
- nil -1 nil
-(te (float-time
-  (org-encode-time
-   (list 0
- (org-element-property :minute-end timestamp)
- (org-element-property :hour-end timestamp)
- (org-element-property :day-end timestamp)
- (org-element-property :month-end timestamp)
- (org-element-property :year-end timestamp)
- nil -1 nil
-(dt (- (if tend (min te tend) te)
-   (if tstart (max ts tstart) ts
-   (when (> dt 0) (cl-incf t1 (floor dt 60)
-((match-end 4)
- ;; A naked time.
- (setq t1 (+ t1 (string-to-number (match-string 5))
- (* 60 (string-to-number (match-string 4))
-((memq element-type '(headline inlinetask)) ;A headline
- ;; Add the currently clocking item time to the total.
- (when (and org-clock-report-include-clocking-task
-(eq (org-clocking-buffer) (current-buffer))
-(eq (marker-position org-clock-hd-marker) (point))
- 

[PATCH] Add new option 'org-imenu-flatten'

2023-12-07 Thread Morgan Smith
* lisp/org/org-compat.el: Add definition of 'org-imenu-flatten'.
(org-imenu-get-tree): Use 'org-imenu-flatten'.
---

Hello!

I've been using this patch for a bit and I quite like it.  One would think
there would be an option in imenu itself to flatten trees but that does not
seem to be the case.  I copied the defcustom from 'doc-view-imenu-flatten'.

Thanks,

Morgan

 lisp/org-compat.el | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index 2697342d6..f2112e4e9 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -1333,6 +1333,10 @@ Pass COLUMN and FORCE to `move-to-column'."
 This also applied for speedbar access."
   :type 'integer)
 
+(defcustom org-imenu-flatten nil
+  "Whether to flatten the list of sections in an imenu or show it nested."
+  :type 'boolean)
+
  Imenu
 
 (defvar-local org-imenu-markers nil
@@ -1353,7 +1357,8 @@ This also applied for speedbar access."
(org-link-display-format (org-get-heading t t t t)
 (when (and (<= level org-imenu-depth) (org-string-nw-p headline))
   (let* ((m (point-marker))
- (item (propertize headline 'org-imenu-marker m 'org-imenu t)))
+ (item (propertize headline 'org-imenu-marker m 'org-imenu t))
+  (level (if org-imenu-flatten 1 level)))
 (push m org-imenu-markers)
 (if (>= level last-level)
 (push (cons item m) (aref subs level))
-- 
2.41.0




Re: [PATCH] Add new option 'org-imenu-flatten'

2023-12-08 Thread Morgan Smith
Ihor Radchenko  writes:

>
> Have you considered adding a "flatten" option to imenu itself?
> That way, you could automatically get the functionality for free
> everywhere, not just in Org mode.

I have considered that but gave up with minimal investigation because it
seemed harder then this solution.  It's possible imenu did actually have
this functionality sometime before 1998 (see commit
fe2908be7b09f4c765ebdaf16fe07b0a77f78ba8).

The doc-view imenu-flatten stuff was added 2022-09-28 (see commit
fe002cc8ce38efb256a2a60660ee626c2b2cdf81).  This makes me feel like
maybe that person thought adding it to imenu directly would be hard.

I might at some point investigate doing that but likely not soon.  Also
if that feature was ever added, it would still be compatible with the
patch I sent.  For those reasons, I advocate my patch should still be
applied even though it is clear that it is a sub-optimal solution.



[PATCH] lisp/org-agenda.el: Check agenda type earlier

2023-12-23 Thread Morgan Smith
lisp/org-agenda.el (org-agenda-goto-date): Check agenda type earlier.
Also remove redundant error.

When this function is run on a todo agenda the user is given the
undescriptive error "(wrong-type-argument listp "todo")" because we
attempt to parse the 'org-last-args text-property prematurely.  With
this change users will get the much better error "Not allowed in
'todo'-type agenda buffer or component".
---
 lisp/org-agenda.el | 48 +++---
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index d4dd6d823..9b75ee943 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -8598,31 +8598,31 @@ See also:
(list
 (let ((org-read-date-prefer-future org-agenda-jump-prefer-future))
   (org-read-date
+  (org-agenda-check-type t 'agenda)
   (let* ((day (time-to-days (org-time-string-to-time date)))
-(org-agenda-sticky-orig org-agenda-sticky)
-(org-agenda-buffer-tmp-name (buffer-name))
-(args (get-text-property (min (1- (point-max)) (point)) 
'org-last-args))
-(0-arg (or current-prefix-arg (car args)))
-(2-arg (nth 2 args))
-(with-hour-p (nth 4 org-agenda-redo-command))
-(newcmd (list 'org-agenda-list 0-arg date
-  (org-agenda-span-to-ndays
-   2-arg (org-time-string-to-absolute date))
-  with-hour-p))
-(newargs (cdr newcmd))
-(inhibit-read-only t)
-org-agenda-sticky)
-(if (not (org-agenda-check-type t 'agenda))
-   (error "Not available in non-agenda views")
-  (add-text-properties (point-min) (point-max)
-  `(org-redo-cmd ,newcmd org-last-args ,newargs))
-  (org-agenda-redo)
-  (goto-char (point-min))
-  (while (not (or (= (or (get-text-property (point) 'day) 0) day)
- (save-excursion (move-beginning-of-line 2) (eobp
-   (move-beginning-of-line 2))
-  (setq org-agenda-sticky org-agenda-sticky-orig
-   org-agenda-this-buffer-is-sticky org-agenda-sticky
+ (org-agenda-sticky-orig org-agenda-sticky)
+ (org-agenda-buffer-tmp-name (buffer-name))
+ (args (get-text-property (min (1- (point-max)) (point))
+  'org-last-args))
+ (0-arg (or current-prefix-arg (car args)))
+ (2-arg (nth 2 args))
+ (with-hour-p (nth 4 org-agenda-redo-command))
+ (newcmd (list 'org-agenda-list 0-arg date
+   (org-agenda-span-to-ndays
+2-arg (org-time-string-to-absolute date))
+   with-hour-p))
+ (newargs (cdr newcmd))
+ (inhibit-read-only t)
+ org-agenda-sticky)
+(add-text-properties (point-min) (point-max)
+ `(org-redo-cmd ,newcmd org-last-args ,newargs))
+(org-agenda-redo)
+(goto-char (point-min))
+(while (not (or (= (or (get-text-property (point) 'day) 0) day)
+(save-excursion (move-beginning-of-line 2) (eobp
+  (move-beginning-of-line 2))
+(setq org-agenda-sticky org-agenda-sticky-orig
+  org-agenda-this-buffer-is-sticky org-agenda-sticky)))
 
 (defun org-agenda-goto-today ()
   "Go to today's date in the agenda buffer.
-- 
2.41.0




[PATCH] * doc/org-manual.org: Fix typo

2023-12-23 Thread Morgan Smith
doc/org-manual.org: 'org-hierarchical-checkbox-statistics' ->
'org-checkbox-hierarchical-statistics'
---
 doc/org-manual.org | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index a5cd9735e..8d74fa332 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -4724,7 +4724,7 @@ If all child checkboxes are checked, the parent checkbox 
is also checked.
 #+cindex: statistics, for checkboxes
 #+cindex: checkbox statistics
 #+cindex: @samp{COOKIE_DATA}, property
-#+vindex: org-hierarchical-checkbox-statistics
+#+vindex: org-checkbox-hierarchical-statistics
 The =[2/4]= and =[1/3]= in the first and second line are cookies
 indicating how many checkboxes present in this entry have been checked
 off, and the total number of checkboxes present.  This can give you an
@@ -4732,7 +4732,7 @@ idea on how many checkboxes remain, even without opening 
a folded
 entry.  The cookies can be placed into a headline or into (the first
 line of) a plain list item.  Each cookie covers checkboxes of direct
 children structurally below the headline/item on which the cookie
-appears[fn:: Set the variable ~org-hierarchical-checkbox-statistics~
+appears[fn:: Set the variable ~org-checkbox-hierarchical-statistics~
 if you want such cookies to count all checkboxes below the cookie, not
 just those belonging to direct children.].  You have to insert the
 cookie yourself by typing either =[/]= or =[%]=.  With =[/]= you get
-- 
2.41.0




[PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

2023-12-30 Thread Morgan Smith
lisp/org-agenda.el (org-agenda-get-scheduled): Consolidate deadline
fetching code.  Don't check if deadline is shown when
'org-agenda-skip-scheduled-if-deadline-is-shown' has a value of
'repeated-after-deadline'.

Currently when 'org-agenda-skip-scheduled-if-deadline-is-shown' has a
value of 'repeated-after-deadline' then there is no effect.  This is
because when 'org-agenda-get-scheduled' is run on later dates, the
previous deadlines are not put in 'deadline-pos'.
---
 lisp/org-agenda.el | 49 +++---
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index a1b2f3dc4..df1e8cb7d 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -6606,25 +6606,26 @@ scheduled items with an hour specification like 
[h]h:mm."
  (futureschedp (> schedule today))
  (habitp (and (fboundp 'org-is-habit-p)
(string= "habit" (org-element-property :STYLE 
el
+  (deadline (and (or 
org-agenda-skip-scheduled-delay-if-deadline
+ 
org-agenda-skip-scheduled-if-deadline-is-shown)
+ (when-let ((timestamp (org-element-property 
:deadline el)))
+   (time-to-days
+(org-timestamp-to-time
+ timestamp)
  (suppress-delay
-  (let ((deadline (and 
org-agenda-skip-scheduled-delay-if-deadline
-(org-element-property
- :raw-value
- (org-element-property :deadline 
el)
-(cond
- ((not deadline) nil)
- ;; The current item has a deadline date, so
- ;; evaluate its delay time.
- ((integerp org-agenda-skip-scheduled-delay-if-deadline)
-  ;; Use global delay time.
-  (- org-agenda-skip-scheduled-delay-if-deadline))
- ((eq org-agenda-skip-scheduled-delay-if-deadline
-  'post-deadline)
-  ;; Set delay to no later than DEADLINE.
-  (min (- schedule
-  (org-agenda--timestamp-to-absolute deadline))
-   org-scheduled-delay-days))
- (t 0
+   (cond
+((not (and deadline 
org-agenda-skip-scheduled-delay-if-deadline)) nil)
+;; The current item has a deadline date, so
+;; evaluate its delay time.
+((integerp org-agenda-skip-scheduled-delay-if-deadline)
+ ;; Use global delay time.
+ (- org-agenda-skip-scheduled-delay-if-deadline))
+((eq org-agenda-skip-scheduled-delay-if-deadline
+ 'post-deadline)
+ ;; Set delay to no later than DEADLINE.
+ (min (- schedule deadline)
+  org-scheduled-delay-days))
+(t 0)))
  (ddays
   (cond
;; Nullify delay when a repeater triggered already
@@ -6661,16 +6662,14 @@ scheduled items with an hour specification like 
[h]h:mm."
 ;; doesn't apply to habits.
 (when (pcase org-agenda-skip-scheduled-if-deadline-is-shown
 ((guard
-  (or (not (memq (line-beginning-position 0) deadline-pos))
+  (or (not deadline)
   habitp))
  nil)
 (`repeated-after-deadline
- (let ((deadline (time-to-days
-   (when (org-element-property :deadline 
el)
- (org-time-string-to-time
-  (org-element-interpret-data
-   (org-element-property :deadline 
el)))
-   (and (<= schedule deadline) (> current deadline
+  (and (<= schedule deadline) (> current deadline)))
+ ((guard
+   (not (memq (line-beginning-position 0) deadline-pos)))
+  nil)
 (`not-today pastschedp)
 (`t t)
 (_ nil))
-- 
2.41.0




Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

2024-01-02 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> lisp/org-agenda.el (org-agenda-get-scheduled): Consolidate deadline
>> fetching code.  Don't check if deadline is shown when
>> 'org-agenda-skip-scheduled-if-deadline-is-shown' has a value of
>> 'repeated-after-deadline'.
>>
>> Currently when 'org-agenda-skip-scheduled-if-deadline-is-shown' has a
>> value of 'repeated-after-deadline' then there is no effect.  This is
>> because when 'org-agenda-get-scheduled' is run on later dates, the
>> previous deadlines are not put in 'deadline-pos'.
>
> May you please provide a detailed reproducer demonstrating the bug you
> are trying to fix? Such reproducer could be a basis of a new test.

See a detailed reproducer attached to this mail.  It has a task defined
as this:

* TODO task
SCHEDULED: <2017-03-06 Mon +2d> DEADLINE: <2017-03-10>

Running said reproducer currently fails as follows (the numbers are the
days of the month).

(string-equal
"06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  
task\n"
"06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  
task\n12\nScheduled: task\n")


As you can see, we expect to not see anything scheduled after the
deadline if 'org-agenda-skip-scheduled-if-deadline-is-shown' is set to
'repeated-after-deadline', however, we actually see that things continue
to be scheduled.  Hence the bug is that that option currently does
nothing.

>From d7e72b9f0189b98d7ae59f0a56d54ffe70583956 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 2 Jan 2024 14:25:07 -0500
Subject: [PATCH] Testing: Add tests for
 'org-agenda-skip-scheduled-if-deadline-is-shown'

* testing/lisp/test-org-agenda.el
(test-org-agenda/org-agenda-skip-scheduled-if-deadline-is-shown): New test.
---
 testing/lisp/test-org-agenda.el | 45 +
 1 file changed, 45 insertions(+)

diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index 409d44192..6618fabc5 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -651,6 +651,51 @@ functions."
   (should (= arg-f-call-cnt 1))
   (should (equal f-called-args '(1 2 3))
 
+(ert-deftest test-org-agenda/org-agenda-skip-scheduled-if-deadline-is-shown ()
+  "Test values for `org-agenda-skip-scheduled-if-deadline-is-shown'."
+  (cl-assert (not org-agenda-sticky) nil "precondition violation")
+  (cl-assert (not (org-test-agenda--agenda-buffers))
+ nil "precondition violation")
+  (dolist (test-time '("2017-03-06" "2017-03-10"))
+(let ((org-agenda-custom-commands
+   '(("f" "no fluff" agenda ""
+  ((org-agenda-overriding-header "")
+   (org-agenda-todo-keyword-format "")
+   (org-agenda-prefix-format "%s")
+   (org-agenda-format-date "%d")
+   (org-agenda-show-all-dates nil)
+  (org-deadline-warning-days 0)
+  (todayp (string= test-time "2017-03-10")))
+  (dolist (org-agenda-skip-scheduled-if-deadline-is-shown (list nil t 'not-today 'repeated-after-deadline))
+(org-test-at-time test-time
+  (org-test-agenda-with-agenda
+   "
+* TODO task
+SCHEDULED: <2017-03-06 Mon +2d> DEADLINE: <2017-03-10>
+"
+   (should
+(string-equal
+ (concat "06\n"
+ "Scheduled: task\n"
+ (if todayp ; We don't show repeats scheduled in the past
+ ""
+   "08\nScheduled: task\n")
+ "10\n"
+ (if (and org-agenda-skip-scheduled-if-deadline-is-shown
+  (not (and (not todayp) (eq org-agenda-skip-scheduled-if-deadline-is-shown 'not-today)))
+  (not (eq org-agenda-skip-scheduled-if-deadline-is-shown 'repeated-after-deadline)))
+ ""
+   (if todayp
+   "Sched. 4x: task\n"
+ "Scheduled: task\n"))
+ "Deadline:  task\n"
+ (unless (eq org-agenda-skip-scheduled-if-deadline-is-shown 'repeated-after-deadline)
+   "12\nScheduled: task\n"))
+ (progn
+   (org-agenda nil "f")
+   (substring-no-properties (buffer-string)
+   (org-test-agenda--kill-all-agendas)))
+
 
 
 (provide 'test-org-agenda)
-- 
2.41.0



Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

2024-01-03 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>>> May you please provide a detailed reproducer demonstrating the bug you
>>> are trying to fix? Such reproducer could be a basis of a new test.
>>
>> See a detailed reproducer attached to this mail.  It has a task defined
>> as this:
>>
>> * TODO task
>> SCHEDULED: <2017-03-06 Mon +2d> DEADLINE: <2017-03-10>
>>
>> Running said reproducer currently fails as follows (the numbers are the
>> days of the month).
>>
>> (string-equal
>> "06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  
>> task\n"
>> "06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  
>> task\n12\nScheduled: task\n")
>>
>> As you can see, we expect to not see anything scheduled after the
>> deadline if 'org-agenda-skip-scheduled-if-deadline-is-shown' is set to
>> 'repeated-after-deadline', however, we actually see that things continue
>> to be scheduled.  Hence the bug is that that option currently does
>> nothing.
>
> I think that you misunderstand the purpose of 'repeated-after-deadline
> value. Let me provide an example:
>
> 'repeated-after-deadline allows this task
>
> * Do me every day before March, 16th (included)
>   SCHEDULED: <2013-03-12 mar. +1d> DEADLINE: <2013-03-16 sam.>
>
> to step being displayed after March, 16th.

In my above example I have a task with a deadline of March 10th.
My test shows that it will continue to be scheduled after March 10th, on
the 12th.  That is wrong.  It is still scheduled on the 10th which is
good.  Being scheduled on the 12th is wrong.



Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

2024-01-16 Thread Morgan Smith
Ihor Radchenko  writes:

> But then `org-agenda-skip-scheduled-if-deadline-is-shown' does not look
> like the right place for this customization. It would make more sense to
> have a dedicated custom variable for this. WDYT?
>
> I am adding Bastien to the loop - he authored the original feature
> implementation.

I can't quite remember at this point but I'm pretty sure some
permutations are impossible with the current setup (like ignoring both
things that are scheduled today and in the future when the deadline is
today).

Regardless, I would like to say: perfect is the enemy of good.  It's a
little disheartening to see my work to stagnate over this.  If we want
to rename stuff, great, lets do that.  Otherwise I would appreciate it
if this work got incorporated.  If nothing else, I would at least like
to see my test accepted as I believe it is a good regression test.  If
you would like I can even remove the 'repeated-after-deadline' stuff and
resubmit it.



Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

2024-01-16 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> Ihor Radchenko  writes:
>>
>>> But then `org-agenda-skip-scheduled-if-deadline-is-shown' does not look
>>> like the right place for this customization. It would make more sense to
>>> have a dedicated custom variable for this. WDYT?
>>> ..
>> I can't quite remember at this point but I'm pretty sure some
>> permutations are impossible with the current setup (like ignoring both
>> things that are scheduled today and in the future when the deadline is
>> today).
>
> Your statement does not look like something related to my question about
> moving 'repeated-after-deadline value to some other (existing or new)
> customization.
>
By adding another variable to control the 'repeated-after-deadline'
behavior, we would allow the user to combine this behavior with the
other behaviors of 'org-agenda-skip-scheduled-if-deadline-is-shown'.
This would allow for more permutations.  This is a point in favor of
adding a dedicated custom variable.
>
> Or did you mean to provide a generic suggestion about adding more agenda
> customizations?
>
Maybe we could create some data-structure that represents an agenda and
allow the user to manipulate that directly.  I'm not really a fan of
having a ton of variables controlling the agenda.  Although I imagine I
might gain some respect for it if I tried to implement this feature.
>
> Nothing is stagnated. I am just waiting for Bastien to chime in, if he
> decides to. He is the person who can provide important context.
> We allow up to one month for people to reply on the mailing list.
> See https://orgmode.org/worg/org-mailing-list.html#org474747a
>
I was unaware of the one month policy.  Thank you for letting me know.
Due to that lack of a bug tracking system I have a fear that things on
the mailing list would get lost.  Maybe this fear is unfounded?



[PATCH] lisp/org-capture.el: Simplify 'org-capture-get-indirect-buffer'

2024-03-31 Thread Morgan Smith
* lisp/org-capture.el (org-capture-get-indirect-buffer): Simplify by
using 'generate-new-buffer-name'.
---
 lisp/org-capture.el | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 8ce11cb75..5d9ae4947 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1585,10 +1585,7 @@ If TEMPLATE-KEY is nil, the user is queried for the 
template."
   "Make an indirect BUFFER for a capture process.
 Use PREFIX as a prefix for the name of the indirect buffer."
   (setq buffer (or buffer (current-buffer)))
-  (let ((n 1) (base (buffer-name buffer)) bname)
-(setq bname (concat prefix "-" base))
-(while (buffer-live-p (get-buffer bname))
-  (setq bname (concat prefix "-" (number-to-string (cl-incf n)) "-" base)))
+  (let ((bname (generate-new-buffer-name (concat prefix "-" (buffer-name 
buffer)
 (condition-case nil
 (make-indirect-buffer buffer bname 'clone)
   (error
-- 
2.41.0




[PATCH] org-manual: Document Org Plot option "timeind"

2024-04-02 Thread Morgan Smith
* doc/org-manual.org (Plot options): Document "timeind".  Also fix the
formatting for a couple other entries.
---
 doc/org-manual.org | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index ad5c85e2a..69c760c8b 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -2991,6 +2991,11 @@  Plot options
 
   Specify which column of the table to use as the =x= axis.
 
+- =timeind= ::
+
+  Specify which column of the table to use as the =x= axis as a time
+  value.
+
 - =deps= ::
 
   Specify the columns to graph as a Lisp style list, surrounded by
@@ -2998,7 +3003,7 @@  Plot options
   the third and fourth columns.  Defaults to graphing all other
   columns aside from the =ind= column.
 
-- transpose ::
+- =transpose= ::
 
   When =y=, =yes=, or =t= attempt to transpose the table data before
   plotting.  Also recognizes the shorthand option =trans=.
@@ -3033,21 +3038,21 @@  Plot options
   When plotting =3d= or =grid= types, set this to =t= to graph a flat
   mapping rather than a =3d= slope.
 
-- min ::
+- =min= ::
 
   Provides a minimum axis value that may be used by a plot type.
   Implicitly assumes the =y= axis is being referred to.  Can
   explicitly provide a value for a either the =x= or =y= axis with
   =xmin= and =ymin=.
 
-- max ::
+- =max= ::
 
   Provides a maximum axis value that may be used by a plot type.
   Implicitly assumes the =y= axis is being referred to.  Can
   explicitly provide a value for a either the =x= or =y= axis with
   =xmax= and =ymax=.
 
-- ticks ::
+- =ticks= ::
 
   Provides a desired number of axis ticks to display, that may be used
   by a plot type.  If none is given a plot type that requires ticks
-- 
2.41.0




[PATCH] lisp/org-element.el: Add repeater-deadline support to org-element

2024-04-03 Thread Morgan Smith
* lisp/org-element.el (org-element-timestamp-parser,
org-element-timestamp-interpreter): Add support for repeater
deadlines.  Adds two new properties: ':repeater-deadline-value' and
':repeater-deadline-unit'.

* testing/lisp/test-org-element.el (test-org-element/timestamp-parser,
test-org-element/timestamp-interpreter): Test support for repeater
deadlines.
---

Hello!

I would like to add some features to org-habit (something I have tried
unsuccessfully in the past).  Before I do that, I would like to switch
org-habit over to the org-element api.  Before I do that, I would like to
extend org-element to support the org-habit syntax.

Let me know what you think!  All the tests pass on my machine.

Thanks,

Morgan

 lisp/org-element.el  | 58 +---
 testing/lisp/test-org-element.el | 38 +++--
 2 files changed, 74 insertions(+), 22 deletions(-)

diff --git a/lisp/org-element.el b/lisp/org-element.el
index f4eec1695..8d3b8ce44 100644
--- a/lisp/org-element.el
+++ b/lisp/org-element.el
@@ -4288,12 +4288,13 @@ Assume point is at the target."
   "Parse time stamp at point, if any.
 
 When at a time stamp, return a new syntax node of `timestamp' type
-containing `:type', `:range-type', `:raw-value', `:year-start', `:month-start',
-`:day-start', `:hour-start', `:minute-start', `:year-end',
-`:month-end', `:day-end', `:hour-end', `:minute-end',
+containing `:type', `:range-type', `:raw-value', `:year-start',
+`:month-start', `:day-start', `:hour-start', `:minute-start',
+`:year-end', `:month-end', `:day-end', `:hour-end', `:minute-end',
 `:repeater-type', `:repeater-value', `:repeater-unit',
-`:warning-type', `:warning-value', `:warning-unit', `:begin', `:end'
-and `:post-blank' properties.  Otherwise, return nil.
+`:repeater-deadline-value', `:repeater-deadline-unit', `:warning-type',
+`:warning-value', `:warning-unit', `:begin', `:end' and `:post-blank'
+properties.  Otherwise, return nil.
 
 Assume point is at the beginning of the timestamp."
   (when (looking-at-p org-element--timestamp-regexp)
@@ -4326,20 +4327,28 @@ Assume point is at the beginning of the timestamp."
   (date-end 'daterange)
   (time-range 'timerange)
   (t nil)))
-(repeater-props
- (and (not diaryp)
-  (string-match "\\([.+]?\\+\\)\\([0-9]+\\)\\([hdwmy]\\)"
-raw-value)
-  (list
-   :repeater-type
-   (let ((type (match-string 1 raw-value)))
- (cond ((equal "++" type) 'catch-up)
-   ((equal ".+" type) 'restart)
-   (t 'cumulate)))
-   :repeater-value (string-to-number (match-string 2 
raw-value))
-   :repeater-unit
-   (pcase (string-to-char (match-string 3 raw-value))
- (?h 'hour) (?d 'day) (?w 'week) (?m 'month) (_ 'year)
+ (repeater-props
+  (and (not diaryp)
+   (string-match
+
"\\([.+]?\\+\\)\\([0-9]+\\)\\([hdwmy]\\)/?\\([0-9]+\\)?\\([hdwmy]\\)?"
+raw-value)
+   (nconc
+(list
+ :repeater-type
+ (let ((type (match-string 1 raw-value)))
+   (cond ((equal "++" type) 'catch-up)
+ ((equal ".+" type) 'restart)
+ (t 'cumulate)))
+ :repeater-value (string-to-number (match-string 2 
raw-value))
+ :repeater-unit
+ (pcase (string-to-char (match-string 3 raw-value))
+   (?h 'hour) (?d 'day) (?w 'week) (?m 'month) (_ 'year)))
+(when (and (match-string 4 raw-value) (match-string 5 
raw-value))
+  (list
+   :repeater-deadline-value (string-to-number 
(match-string 4 raw-value))
+   :repeater-deadline-unit
+   (pcase (string-to-char (match-string 5 raw-value))
+ (?h 'hour) (?d 'day) (?w 'week) (?m 'month) (_ 
'year)))
 (warning-props
  (and (not diaryp)
   (string-match "\\(-\\)?-\\([0-9]+\\)\\([hdwmy]\\)" raw-value)
@@ -4407,7 +4416,16 @@ Assume point is at the beginning of the timestamp."
 (let ((val (org-element-property :repeater-value 
timestamp)))
   (and val (number-to-string val)))
 (pcase (org-element-property :repeater-unit timestamp)
-  (`hour "h") (`day "d") (`week "w") (`month "m") (`year 
"y"
+  (`hour "h") (`day "d") (`week "w") (`month "m") (`year 
"y"))
+ (let ((deadline-value (org-element-property 
:repeater-deadline-value timestamp))
+   (dea

Re: [PATCH] lisp/org-element.el: Add repeater-deadline support to org-element

2024-04-04 Thread Morgan Smith
Hello,

Thanks for the review!

See two patches attached.  One for org and one for worg.  Tests still
pass on my end.

Ihor Radchenko  writes:

> Please also add etc/ORG-NEWS entry - it is org-element API change.

Done

> In addition to changes in Org git, you also need to update
> https://orgmode.org/worg/org-syntax.html#Timestamps and
> https://orgmode.org/worg/dev/org-element-api.html(the page source is at 
> https://git.sr.ht/~bzg/worg)

Done.  Sort of.  My change in org-syntax.org now implies that a repeater
must come before a delay.  I don't know what syntax to use that doesn't
make that implication.  Although I don't see the harm in telling people
to put the repeater first.

> This will match timestamps like <2012-03-29 Thu +1y2y>. You may instead
> use shy group \(?:...\)? around the whole /2y regexp match. (Or even
> rewrite the regexp via rx for better readability, while we are on it).

Done.  It's my first time using rx though.  I don't know if I should be
compiling it or something for performance?

> This is slightly confusing, because "deadline" has multiple meanings in
> Org. repeater-deadline-value/unit would be more readable as the variable name.

Done

>From b285023ca107f7c0fc8b89f7f636cf96bd217207 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Thu, 4 Apr 2024 16:49:31 -0400
Subject: [PATCH] Document repeater deadline syntax and element api

* dev/org-element-api.org (Timestamp): Add ':repeater-deadline-unit'
and ':repeater-deadline-value'.
* org-syntax.org (Timestamps): Separate definition of repeater and
delay.  Add repeater deadline to repeater definition.
---
 dev/org-element-api.org |  5 +
 org-syntax.org  | 32 +++-
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/dev/org-element-api.org b/dev/org-element-api.org
index ffcda274..9d57238b 100644
--- a/dev/org-element-api.org
+++ b/dev/org-element-api.org
@@ -706,6 +706,11 @@ ** Timestamp
   (symbol: ~year~, ~month~, ~week~, ~day~, ~hour~ or ~nil~).
 - ~:repeater-value~ :: Value of shift, if a repeater is defined
   (integer or ~nil~).
+- ~:repeater-deadline-unit~ :: Unit of shift, if a repeater deadline
+  is defined (symbol: ~year~, ~month~, ~week~, ~day~, ~hour~ or
+  ~nil~).
+- ~:repeater-deadline-value~ :: Value of shift, if a repeater deadline
+  is defined (integer or ~nil~).
 - ~:type~ :: Type of timestamp (symbol: ~active~, ~active-range~,
   ~diary~, ~inactive~, ~inactive-range~).
 - ~:range-type~ :: Type of range (symbol: ~daterange~, ~timerange~ or
diff --git a/org-syntax.org b/org-syntax.org
index bdd372d1..cf477655 100644
--- a/org-syntax.org
+++ b/org-syntax.org
@@ -1743,13 +1743,13 @@ ** Timestamps
 Timestamps are structured according to one of the seven following patterns:
 
 #+begin_example
-<%%(SEXP)> (diary)
-  (active)
-[DATE TIME REPEATER-OR-DELAY]  (inactive)
---   (active range)
- (active range)
-[DATE TIME REPEATER-OR-DELAY]--[DATE TIME REPEATER-OR-DELAY]   (inactive range)
-[DATE TIME-TIME REPEATER-OR-DELAY] (inactive range)
+<%%(SEXP)>   (diary)
+   (active)
+[DATE TIME REPEATER DELAY]   (inactive)
+--   (active range)
+  (active range)
+[DATE TIME REPEATER DELAY]--[DATE TIME REPEATER DELAY]   (inactive range)
+[DATE TIME-TIME REPEATER DELAY]  (inactive range)
 #+end_example
 
 + SEXP :: A string consisting of any characters but =>= and =\n=.
@@ -1763,20 +1763,26 @@ ** Timestamps
 + TIME (optional) :: An instance of the pattern =H:MM= where =H=
   represents a one to two digit number (and can start with =0=), and =M=
   represents a single digit.
-+ REPEATER-OR-DELAY (optional) :: An instance of the following pattern:
++ REPEATER (optional) :: An instance of the following pattern:
   #+begin_example
 MARK VALUE UNIT
+MARK VALUE UNIT/VALUE UNIT
   #+end_example
   Where MARK, VALUE and UNIT are not separated by whitespace characters.
   - MARK :: Either the string =+= (cumulative type), =++= (catch-up type),
-or =.+= (restart type) when forming a repeater, and either =-= (all
-type) or =--= (first type) when forming a warning delay.
+or =.+= (restart type).
+  - VALUE :: A number
+  - UNIT :: Either the character =h= (hour), =d= (day), =w= (week), =m=
+(month), or =y= (year)
++ DELAY (optional) :: An instance of the following pattern:
+  #+begin_example
+MARK VALUE UNIT
+  #+end_example
+  Where MARK, VALUE and UNIT are not separated by whitespace characters.
+  - MARK :: Either  =-= (all type) or =--= (first type).
   - VALUE :: A number
   - UNIT :: Either the character =h= (hour), =d= (day), =w= (week), =m=

Re: [PATCH] lisp/org-element.el: Add repeater-deadline support to org-element

2024-04-10 Thread Morgan Smith
Apologies for the delay.  I had lots of social events surrounding the
solar eclipse.  The eclipse was really cool, I do recommend.

See two patches attached again.  All tests pass on my computer.

I decided to add an extra `let' statement to my changes to
`org-element-timestamp-parser'.  I think it adds a touch of clarity and
maybe performance.

Ihor Radchenko  writes:

>> My change in org-syntax.org now implies that a repeater
>> must come before a delay.  I don't know what syntax to use that doesn't
>> make that implication.  Although I don't see the harm in telling people
>> to put the repeater first.
>
> We can simply leave the previous REPEATER-OR-DELAY, but expand on it
> that REPEATER-OR-DELAY is an instance of REPEATER or an instance of
> DELAY. Does it make sense?

Done.

>> Done.  It's my first time using rx though.  I don't know if I should be
>> compiling it or something for performance?
>
> `rx' is a macro. It will be expanded during compilation.

Thank you for the piece of mind.  I'll have to use `rx' more often :).


Thanks for the reviews!  I'll start switching org-habit over to the
element API soon.  I've already played with that a bit

>From 582d4e7372c005f098f213b496de6f85c0c11d2f Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 3 Apr 2024 16:30:42 -0400
Subject: [PATCH] lisp/org-element.el: Add repeater-deadline support to
 org-element

* lisp/org-element.el (org-element-timestamp-parser,
org-element-timestamp-interpreter): Add support for repeater
deadlines.  Adds two new properties: ':repeater-deadline-value' and
':repeater-deadline-unit'.

* testing/lisp/test-org-element.el (test-org-element/timestamp-parser,
test-org-element/timestamp-interpreter): Test support for repeater
deadlines.

* etc/ORG-NEWS: Add relevant news.
---
 etc/ORG-NEWS | 14 +++
 lisp/org-element.el  | 70 +++-
 testing/lisp/test-org-element.el | 38 -
 3 files changed, 100 insertions(+), 22 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index aeb7ffd4b..2b418cd3c 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -512,6 +512,20 @@ timestamp object.  Possible values: ~timerange~, ~daterange~, ~nil~.
 ~org-element-timestamp-interpreter~ takes into account this property
 and returns an appropriate timestamp string.
 
+ New properties =:repeater-deadline-value= and =:repeater-deadline-unit= for org-element timestamp object
+
+~org-element-timestamp-parser~ now adds =:repeater-deadline-value= and
+=:repeater-deadline-unit= properties to each timestamp object that has
+a repeater deadline.
+
+Possible values for =:repeater-deadline-value=: ~positive integer~, ~nil~.
+
+Possible values for =:repeater-deadline-unit=: ~hour~, ~day~, ~week~,
+~month~, ~year~.
+
+~org-element-timestamp-interpreter~ takes into account these properties
+and returns an appropriate timestamp string.
+
  =org-link= store functions are passed an ~interactive?~ argument
 
 The ~:store:~ functions set for link types using
diff --git a/lisp/org-element.el b/lisp/org-element.el
index 8e5416d8b..49a312694 100644
--- a/lisp/org-element.el
+++ b/lisp/org-element.el
@@ -4288,12 +4288,13 @@ Assume point is at the target."
   "Parse time stamp at point, if any.
 
 When at a time stamp, return a new syntax node of `timestamp' type
-containing `:type', `:range-type', `:raw-value', `:year-start', `:month-start',
-`:day-start', `:hour-start', `:minute-start', `:year-end',
-`:month-end', `:day-end', `:hour-end', `:minute-end',
+containing `:type', `:range-type', `:raw-value', `:year-start',
+`:month-start', `:day-start', `:hour-start', `:minute-start',
+`:year-end', `:month-end', `:day-end', `:hour-end', `:minute-end',
 `:repeater-type', `:repeater-value', `:repeater-unit',
-`:warning-type', `:warning-value', `:warning-unit', `:begin', `:end'
-and `:post-blank' properties.  Otherwise, return nil.
+`:repeater-deadline-value', `:repeater-deadline-unit', `:warning-type',
+`:warning-value', `:warning-unit', `:begin', `:end' and `:post-blank'
+properties.  Otherwise, return nil.
 
 Assume point is at the beginning of the timestamp."
   (when (looking-at-p org-element--timestamp-regexp)
@@ -4326,20 +4327,38 @@ Assume point is at the beginning of the timestamp."
   (date-end 'daterange)
   (time-range 'timerange)
   (t nil)))
-	 (repeater-props
-	  (and (not diaryp)
-		   (string-match "\\([.+]?\\+\\)\\([0-9]+\\)\\([hdwmy]\\)"
- raw-value)
-		   (list
-		:repeater-type
-		(let ((type (match-s

[PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx

2024-04-11 Thread Morgan Smith
Hello!

See two attached patches.  All tests pass on my computer.

Every once in a while I feel obligated to go back to org-clock-sum to
try and optimize it.  I have a file with 8 clocktables in it and it
takes forever to update.  This time I decided instead of trying to
optimize, I'm just going to try and understand.

The regex has been altered slightly.

1. Instead of using "[ \t]", I decided to use [[:blank:]].  No real
reason.  I just think it's easier to read and maybe slightly more
correct?

2. For the timestamps, instead of ".*?" (using a non-greedy ".*") I
decided to use "[^]]*" (accept everything except "]").  I did this simply
because I'm not used to using non-greedy regex's.  Maybe this way
performs better?  I didn't test that.

3. I used the variable `org-outline-regexp' but that doesn't actually
change the regex.

>From 3c3d7abed25cafb2be1096ca079a0e8be907c644 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Thu, 11 Apr 2024 12:23:21 -0400
Subject: [PATCH 1/2] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx

---
 lisp/org-clock.el | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 65a54579a..5ef987ab8 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -2008,9 +2008,23 @@ each headline in the time range with point at the headline.  Headlines for
 which HEADLINE-FILTER returns nil are excluded from the clock summation.
 PROPNAME lets you set a custom text property instead of :org-clock-minutes."
   (with-silent-modifications
-(let* ((re (concat "^\\(\\*+\\)[ \t]\\|^[ \t]*"
-		   org-clock-string
-		   "[ \t]*\\(?:\\(\\[.*?\\]\\)-+\\(\\[.*?\\]\\)\\|=>[ \t]+\\([0-9]+\\):\\([0-9]+\\)\\)"))
+(let* ((re (rx line-start
+   (or
+(group (regexp org-outline-regexp))
+(seq (* blank)
+ (literal org-clock-string)
+ (* blank)
+ (or
+  (seq
+   (group "[" (* (not "]")) "]")
+   (+ "-")
+   (group "[" (* (not "]")) "]"))
+  (seq
+   "=>"
+   (+ blank)
+   (group (+ digit))
+   ":"
+       (group (+ digit
 	   (lmax 30)
 	   (ltimes (make-vector lmax 0))
 	   (level 0)
-- 
2.41.0

>From e5298920568e4c5a34589640f11edfa09a98d0d1 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Thu, 11 Apr 2024 12:51:18 -0400
Subject: [PATCH 2/2] Test clock times without timestamps

* testing/lisp/test-org-clock.el (test-org-clock/clocktable/insert):
Add a clock time that does not include timestamps.
---
 testing/lisp/test-org-clock.el | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 44c62e7bc..be8acb529 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -345,13 +345,12 @@ CLOCK: [2022-11-03 %s 06:00]--[2022-11-03 %s 06:01] =>  0:01
(equal
 "| Headline | Time   |
 |--+|
-| *Total time* | *1:00* |
+| *Total time* | *2:00* |
 |--+|
-| H1   | 1:00   |"
+| H1   | 2:00   |"
 (org-test-with-temp-text "* H1\n"
-  (insert (org-test-clock-create-clock ". 1:00" ". 2:00"))
-
-  (goto-line 2)
+  (insert (org-test-clock-create-clock ". 1:00" ". 2:00")
+  "CLOCK: => 1:00\n")
   (require 'org-clock)
   (org-dynamic-block-insert-dblock "clocktable")
 
-- 
2.41.0



Re: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx

2024-04-13 Thread Morgan Smith
Ihor Radchenko  writes:

>> * testing/lisp/test-org-clock.el (test-org-clock/clocktable/insert):
>> Add a clock time that does not include timestamps.
>> ...
>> -
>> -  (goto-line 2)
>> +  (insert (org-test-clock-create-clock ". 1:00" ". 2:00")
>> +  "CLOCK: => 1:00\n")
>
> This is not a valid clock format. Matching such lines is a bug.
> See https://list.orgmode.org/orgmode/87wpkkhafc.fsf@saiph.selenimh/

Let me preface this defense with the fact that I don't like this format
and I don't think we should support it.  Rewriting `org-clock-sum' would
be much easier if we drop support for it.  However, I do believe we
currently support it.

First of all, it currently does work.

Accord to the "Version 4.78" release notes as found on worg, this is
valid.

```
   - You may specify clocking times by hand (i.e. without
 clocking in and out) using this syntax.

 : CLOCK: => 2:00

 Thanks to Scott Jaderholm for this proposal.
```

Also last time I went to rewrite `org-clock-sum' you said
(https://list.orgmode.org/orgmode/87bkg7xbxo.fsf@localhost/):

```
Further, you dropped the

 ((match-end 4)
  ;; A naked time.

branch of the code, which accounts for CLOCK: => HH:MM lines that are not clock 
elements.
```



Re: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx

2024-04-13 Thread Morgan Smith
Ihor Radchenko  writes:

> So, in the message I linked, Nicolas (the major Org mode contributor)
> was not right. I hence need to fix the parser and update Org syntax
> page. This includes fixing `org-element-clock-line-re' to account for
> CLOCK: => 1:00 syntax.

Cool.  I guess ping this thread when that's done so I can give you
another version of the patch.  Or if you'd like help with that stuff let
me know.  I'm here to help.



`org-element-cache-map' misses elements at end of buffer

2024-04-18 Thread Morgan Smith
Hello!

I may or may not have been trying to rewrite `org-clock-sum' for like
the 5th time.

Anyways I was wanting to parse a file using `org-element-cache-map' that
looked roughly like this:


* clock
:LOGBOOK:
CLOCK: [2024-03-24 Sun 15:18]--[2024-03-24 Sun 15:39] =>  0:21
:END:
===

However I couldn't get the clock element this way.  After much trail and
error I found that simply adding another heading on the end allowed me
to get the clock element!

I've attached a test so you can reproduce the problem.  In the test it
is a paragraph element that is not showing up.

>From 9deff2111b73bb2ceb9127db5d88486affa04f0b Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Thu, 18 Apr 2024 09:18:51 -0400
Subject: [PATCH] failing test

---
 testing/lisp/test-org-element.el | 13 +
 1 file changed, 13 insertions(+)

diff --git a/testing/lisp/test-org-element.el b/testing/lisp/test-org-element.el
index 8b2cf1642..313556f49 100644
--- a/testing/lisp/test-org-element.el
+++ b/testing/lisp/test-org-element.el
@@ -4983,6 +4983,19 @@ Text
 
 
 ;;; Test Cache.
+(ert-deftest test-org-element/cache-map ()
+  "Test `org-element-cache-map'."
+  (org-test-with-temp-text "* headline\n:DRAWER:\nparagraph\n:END:\n* headline 2"
+(should
+ (equal
+  '(org-data headline section drawer paragraph headline)
+  (org-element-cache-map #'car :granularity 'element
+  (org-test-with-temp-text "* headline\n:DRAWER:\nparagraph\n:END:"
+(should
+ (equal
+  ;; This fails because paragraph isn't found!
+  '(org-data headline section drawer paragraph)
+  (org-element-cache-map #'car :granularity 'element)
 
 (ert-deftest test-org-element/cache ()
   "Test basic expectations and common pitfalls for cache."
-- 
2.41.0



Re: `org-element-cache-map' misses elements at end of buffer

2024-04-19 Thread Morgan Smith
So I found another bug in `org-element-cache-map'.

Executing the following code just freezes up.  I am struggling to work
through the logic of `org-element-cache-map'.  If no-one else magically
solves my issues, I'll figure it out eventually.  But I would appreciate
some advice on how to debug this stuff (both my issue of missing
elements and freezing).

I guess my next current step is to read the manual on breakpoints.  I'm
getting a little tired of just shoving print statements everywhere.

#+BEGIN_SRC elisp
;; I get that this use case looks weird but in a more complicated
;; situation I also experienced freezing so I'm hoping solving this case
;; will also solve my other situation.
(require 'org-test)
(org-test-with-temp-text "* headline\nCLOCK: => 23:43\n"
(let ((headline (progn (goto-char (point-min)) (org-element-at-point
 (org-element-cache-map
 #'car
 :granularity 'element
 :next-re org-element-clock-line-re
 :after-element headline)))
#+END_SRC



Re: `org-element-cache-map' misses elements at end of buffer

2024-04-21 Thread Morgan Smith
Ihor Radchenko  writes:

>
> Hmm. :after-element keyword logic is broken. It does not account for the
> case when :after-element is past the START point.
>
> It is the time to refactor this function yet again.
> (a tricky endeavour considering all the edge cases we can encounter when
> there are changes in buffer while `org-element-cache-map' is mapping
> over it).

See attached for a way to break :from-pos as well.  I would like to help
refactor but studying this function is a little dizzying for me.


>From eb37560b9c94bed6e91d6834462173d0a6d7d44b Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Fri, 19 Apr 2024 10:46:28 -0400
Subject: [PATCH] test-org-element/cache-map: Add test for :from-pos

* testing/lisp/test-org-element.el (test-org-element/cache-map): Add
test for :from-pos.
---
 testing/lisp/test-org-element.el | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/testing/lisp/test-org-element.el b/testing/lisp/test-org-element.el
index ad4ff1ce9..a0dbc726e 100644
--- a/testing/lisp/test-org-element.el
+++ b/testing/lisp/test-org-element.el
@@ -4994,7 +4994,21 @@ Text
(equal
 '(org-data headline section drawer paragraph)
 (org-test-with-temp-text "* headline\n:DRAWER:\nparagraph\n:END:"
-  (org-element-cache-map #'car :granularity 'element)
+  (org-element-cache-map #'car :granularity 'element
+  (should
+   (equal
+;; This fails returning '(paragraph paragraph) because it returns
+;; the paragraph under "headline".  This is because at
+;; org-element.el:8028 we travel up the parents of drawer trying
+;; to find a paragraph.
+'(paragraph)
+(org-test-with-temp-text "* headline\nparagraph\n** subheading\n:DRAWER:\nparagraph\n:END:\n"
+  (let ((subheading (org-element-at-point)))
+(org-element-cache-map
+ #'car
+ :granularity 'element
+ :restrict-elements '(paragraph)
+ :from-pos (org-element-contents-begin subheading)))
 
 (ert-deftest test-org-element/cache ()
   "Test basic expectations and common pitfalls for cache."
-- 
2.41.0



[PATCH] Rewrite `org-clock-sum'

2024-04-27 Thread Morgan Smith
Hello!

I may have rewritten org-clock-sum yet again.  See attached patch.

* things I want you to tell me
1. Does this look like something that could be eventually merged upstream or am
   I wasting my time?
2. Would you like me to do more performance testing?  I basically only tested
   my use case.  If yes, should I create some test files for benchmarking that
   can be shared?
3. Do you want `org-element-cache-map' fixed before we merge this patch?  If
   yes, please be willing to wait.  I have already spent probably about 8 hours
   looking into it and it still makes my head hurt.

* todo
The patch is like 95% done.  I still gotta

1. Write a decent docstring for `org-clock-ranges'.  Maybe add a news entry for
   it too.

2. Check `org-clock-hd-marker' for open clock.

3. Figure out what to do about open clocks that aren't the current
   one. Historically we ignored them so I guess I should just do that.

4. Maybe test clocking in inlinetasks.  I honestly don't even know what these
   are.

* Benefits of my rewrite

1. New function `org-clock-ranges' which should help third party packages with
   clock range visualization stuff

2. Performance (see table below)
   - We run the filter before doing all the clock range calculations unlike
 before so aggressive filters should run much faster (I didn't test this
 though).

3. Code is easier to understand (subjective)

* Downsides of my rewrite

1. Does it still perform better with the cache disabled?  idk.  Probably not.

2. Radical change.  Likely has bugs

3. Dances around bugs in `org-element-cache-map' but does it actually dance
   around all of them?

* Performance
I didn't see a big difference on the third run so I assume run 1 is with a cold
cache (obtained by running `org-element-cache-reset') and run 2 is with a warm
cache.

I have an almost 3M file of clocking data.  In it I have this source block
which I use to update my 10 clocktables:

#+BEGIN_SRC elisp
(let (;; (gc-cons-threshold (* 50 1000 1000))
  (start-time (current-time)))
  (org-dblock-update t)
  (time-to-seconds (time-since start-time)))
#+END_SRC

The time results are as follows


| patch   | run # | gc-cons-threshold | time (s) |
|-+---+---+--|
| origin/main | 1 |80 | 59.824324488 |
| mine| 1 |80 | 33.397901059 |
| origin/main | 2 |80 | 48.354095581 |
| mine| 2 |80 | 23.581749901 |
| origin/main | 1 |  5000 | 41.856530738 |
| mine| 1 |  5000 | 30.237918254 |
| origin/main | 2 |  5000 | 33.944309156 |
| mine| 2 |  5000 |  19.84887913 |

>From bfc01710186be01aab2186762cf678d360c5476e Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Thu, 11 Apr 2024 12:23:21 -0400
Subject: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite using element api

---
 lisp/org-clock.el | 191 +++---
 1 file changed, 94 insertions(+), 97 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 65a54579a..8731d6ee5 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -33,15 +33,13 @@
 
 (require 'cl-lib)
 (require 'org)
+(require 'org-element)
 
 (declare-function calendar-iso-to-absolute "cal-iso" (date))
 (declare-function notifications-notify "notifications" (&rest params))
 (declare-function org-element-property "org-element-ast" (property node))
-(declare-function org-element-contents-end "org-element" (node))
-(declare-function org-element-end "org-element" (node))
 (declare-function org-element-type "org-element-ast" (node &optional anonymous))
 (declare-function org-element-type-p "org-element-ast" (node types))
-(defvar org-element-use-cache)
 (declare-function org-inlinetask-at-task-p "org-inlinetask" ())
 (declare-function org-inlinetask-goto-beginning "org-inlinetask" ())
 (declare-function org-inlinetask-goto-end "org-inlinetask" ())
@@ -1998,6 +1996,9 @@ With prefix arg SELECT, offer recently clocked tasks for selection."
 (org-clock-sum (car r) (cadr r)
 		   headline-filter (or propname :org-clock-minutes-custom
 
+;;; TODO:
+;; Maybe add more tests?
+;; Are there tests for inlinetasks?
 ;;;###autoload
 (defun org-clock-sum (&optional tstart tend headline-filter propname)
   "Sum the times for each subtree.
@@ -2008,100 +2009,62 @@ each headline in the time range with point at the headline.  Headlines for
 which HEADLINE-FILTER returns nil are excluded from the clock summation.
 PROPNAME lets you set a custom text property instead of :org-clock-minutes."
   (with-silent-modifications
-(let* ((re (concat "^\\(\\*+\\)[ \t]\\|^[ \t]*"
-		   org-clock-string
-		   "[ \t]

[PATCH] org-make-tags-matcher: Compile returned function

2024-04-28 Thread Morgan Smith
This should result in a nice performance boost when the function is
called repeatedly (as is often done).

* lisp/org.el (org-make-tags-matcher): Evaluate returned function to
compile it into a closure.
---

Hello!

All tests pass.

I don't have any rigorous benchmarks but this does make things significantly
faster.  This actually seems to have a bigger performance impact on
`org-clock-sum' then my rewrite of `org-clock-sum' I submitted earlier does.
Which is a little frustrating honestly.

It does involve using the `eval' function which I know is a little taboo.
Although in this case I don't believe it actually adds any danger.

Thanks,

Morgan

 lisp/org.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index 084132fb3..80ffeeccf 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11726,7 +11726,7 @@ See also `org-scan-tags'."
 (or tagsmatcher todomatcher t
   (when org--matcher-tags-todo-only
(setq matcher `(and (member todo org-not-done-keywords) ,matcher)))
-  (cons match0 `(lambda (todo tags-list level) ,matcher)
+  (cons match0 (eval `(lambda (todo tags-list level) ,matcher) t)
 
 (defun org--tags-expand-group (group tag-groups expanded)
   "Recursively expand all tags in GROUP, according to TAG-GROUPS.
-- 
2.41.0




[PATCH] lisp/org.el: Obsolete `org-cached-entry-get' in favor of `org-entry-get'

2024-04-28 Thread Morgan Smith
* lisp/org.el (org-cached-entry-get): Rewrite in terms
`org-entry-get'.  Obsolete in favor of `org-entry-get'.
(org-make-tags-matcher): Replace uses of `org-cached-entry-get' with
`org-entry-get'.
---
Hello!

All tests pass.

I don't think we can justify the existence of this function but let me know if
I'm wrong.

The caching mechanism used here is likely to cause hard to diagnose issues.

All of the logic here already exists in `org-entry-get'.

This function is mentioned in very few commits unlike its more popular sibling.

Thanks,

Morgan

 lisp/org.el | 21 +
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index c91029c7f..80ffeeccf 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11480,21 +11480,10 @@ are also TODO tasks."
 
 (defalias 'org-tags-sparse-tree 'org-match-sparse-tree)
 
-(defvar org-cached-props nil)
 (defun org-cached-entry-get (pom property)
-  (if (or (eq t org-use-property-inheritance)
- (and (stringp org-use-property-inheritance)
-  (let ((case-fold-search t))
-(string-match-p org-use-property-inheritance property)))
- (and (listp org-use-property-inheritance)
-  (member-ignore-case property org-use-property-inheritance)))
-  ;; Caching is not possible, check it directly.
-  (org-entry-get pom property 'inherit)
-;; Get all properties, so we can do complicated checks easily.
-(cdr (assoc-string property
-  (or org-cached-props
-  (setq org-cached-props (org-entry-properties pom)))
-  t
+  (org-entry-get pom property 'selective))
+
+(make-obsolete 'org-cached-entry-get "use `org-entry-get' instead." "9.7")
 
 (defun org-global-tags-completion-table (&optional files)
   "Return the list of all tags in all agenda buffer/files.
@@ -11670,7 +11659,7 @@ See also `org-scan-tags'."
   ("CATEGORY"
'(org-get-category (point)))
   ("TODO" 'todo)
-  (p `(org-cached-entry-get nil ,p
+  (p `(org-entry-get (point) ,p 'selective
 ;; Determine operand (aka. property
 ;; value).
 (pv (match-string 8 term))
@@ -11707,7 +11696,7 @@ See also `org-scan-tags'."
  (setq term rest)))
  (push `(and ,@tagsmatcher) orlist)
  (setq tagsmatcher nil))
-   (setq tagsmatcher `(progn (setq org-cached-props nil) (or ,@orlist)
+   (setq tagsmatcher `(or ,@orlist
 
 ;; Make the TODO matcher.
 (when (org-string-nw-p todomatch)
-- 
2.41.0




Re: [PATCH] lisp/org.el: Obsolete `org-cached-entry-get' in favor of `org-entry-get'

2024-04-29 Thread Morgan Smith
Ihor Radchenko  writes:

>
>
> This function should yield speedups when matching special properties
> like "CLOCKSUM", "CLOCKSUM_T", "TIMESTAMP", or "TIMESTAMP_IA".
>
> For example, when the requested match tests these properties multiple
> times.
>
> We need a real-life justification, not a theoretical one.
>
> Canceled.

I agree with all your points and I'm sorry for sending noise.  However, I think
I actually found a real-life justification.

It turns out that by replacing `org-cached-entry-get' with
`org-entry-get' the performance of my `org-clock-sum' calls got much
better for my specific use case.  Due to benchmarking with my local
changes in place (sorry), I accidentally attributed this performance
increase to byte-compiling the return of `org-make-tags-matcher'.

These numbers are also with my `org-clock-sum' rewrite patch applied.
They are in a 3M file of almost exclusivly clocking data.  The filters I
use are "CATEGORY={blah}" for one clock table and "-ignore-ITEM={foo}"
for 9 others.  

org-cached-entry-get
1st run: 26.868990287
2nd run: 16.043983143

org-entry-get
1st run: 18.209056578
2nd run: 5.003186764



Re: [PATCH] lisp/org.el: Obsolete `org-cached-entry-get' in favor of `org-entry-get'

2024-05-01 Thread Morgan Smith
Ihor Radchenko  writes:
>
> However, please move the obsolete function definition to org-compat
> instead of removing it completely. We do it to avoid unexpected breakage
> for people and libraries who happen to use this public function.

Done.  See attached

> Also, with the old approach, if you observe slowdowns, you likely have
> some property being calculated slowly (like BLOCKED in my case). Do you
> happen to know which property is it for your setup?

According to my profiler, I think it's using 30% of the CPU time during
my custom org-clock-sum just to get ITEM.  I suppose it's because it
thinks it has to grab and cache everything when all I'm after is ITEM.
I don't see anything else that looks suspicious in the profiler so I
suspect you're seeing a much worse case then I am.  I'll copy paste my
previous performance numbers here again just so you can see my slowdown
is only between 1.5x and 3x.


org-cached-entry-get
1st run: 26.868990287
2nd run: 16.043983143

org-entry-get
1st run: 18.209056578
2nd run: 5.003186764


>From 5d9cef1250ef1eb656b84d3168ebfecb0e9c9c5c Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 1 May 2024 12:36:40 -0400
Subject: [PATCH] Obsolete `org-cached-entry-get' in favor of `org-entry-get'

We have a better performing cache mechanism in `org-entry-get'.

* lisp/org.el (org-make-tags-matcher): Replace uses of
`org-cached-entry-get' with `org-entry-get'.
(org-cached-entry-get): Move to ...
* lisp/org-compat.el (org-cached-entry-get): ... here.  Obsolete in
favor of `org-entry-get'.
---
 lisp/org-compat.el | 20 
 lisp/org.el| 20 ++--
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index 11eb905ee..b73cfc910 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -649,6 +649,26 @@ Counting starts at 1."
 (define-obsolete-variable-alias 'org-plantuml-executable-args 'org-plantuml-args
   "Org 9.6")
 
+(defvar org-cached-props nil)
+(defun org-cached-entry-get (pom property)
+  (if (or (eq t org-use-property-inheritance)
+	  (and (stringp org-use-property-inheritance)
+	   (let ((case-fold-search t))
+		 (string-match-p org-use-property-inheritance property)))
+	  (and (listp org-use-property-inheritance)
+	   (member-ignore-case property org-use-property-inheritance)))
+  ;; Caching is not possible, check it directly.
+  (org-entry-get pom property 'inherit)
+;; Get all properties, so we can do complicated checks easily.
+(cdr (assoc-string property
+		   (or org-cached-props
+			   (setq org-cached-props (org-entry-properties pom)))
+		   t
+
+(make-obsolete 'org-cached-entry-get
+   "Performs badly.  Instead use `org-entry-get' with the argument INHERIT set to `selective'"
+   "9.7")
+
 (defconst org-latex-line-break-safe "[0pt]"
   "Linebreak protecting the following [...].
 
diff --git a/lisp/org.el b/lisp/org.el
index 2d1a2055f..36b52b0ab 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11480,22 +11480,6 @@ are also TODO tasks."
 
 (defalias 'org-tags-sparse-tree 'org-match-sparse-tree)
 
-(defvar org-cached-props nil)
-(defun org-cached-entry-get (pom property)
-  (if (or (eq t org-use-property-inheritance)
-	  (and (stringp org-use-property-inheritance)
-	   (let ((case-fold-search t))
-		 (string-match-p org-use-property-inheritance property)))
-	  (and (listp org-use-property-inheritance)
-	   (member-ignore-case property org-use-property-inheritance)))
-  ;; Caching is not possible, check it directly.
-  (org-entry-get pom property 'inherit)
-;; Get all properties, so we can do complicated checks easily.
-(cdr (assoc-string property
-		   (or org-cached-props
-			   (setq org-cached-props (org-entry-properties pom)))
-		   t
-
 (defun org-global-tags-completion-table (&optional files)
   "Return the list of all tags in all agenda buffer/files.
 Optional FILES argument is a list of files which can be used
@@ -11670,7 +11654,7 @@ See also `org-scan-tags'."
    ("CATEGORY"
 '(org-get-category (point)))
    ("TODO" 'todo)
-   (p `(org-cached-entry-get nil ,p
+   (p `(org-entry-get (point) ,p 'selective
 			 ;; Determine operand (aka. property
 			 ;; value).
 			 (pv (match-string 8 term))
@@ -11707,7 +11691,7 @@ See also `org-scan-tags'."
 	  (setq term rest)))
 	  (push `(and ,@tagsmatcher) orlist)
 	  (setq tagsmatcher nil))
-	(setq tagsmatcher `(progn (setq org-cached-props nil) (or ,@orlist)
+	(setq tagsmatcher `(or ,@orlist
 
 ;; Make the TODO matcher.
 (when (org-string-nw-p todomatch)
-- 
2.41.0



Re: [PATCH] lisp/org.el: Obsolete `org-cached-entry-get' in favor of `org-entry-get'

2024-05-01 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>>> Also, with the old approach, if you observe slowdowns, you likely have
>>> some property being calculated slowly (like BLOCKED in my case). Do you
>>> happen to know which property is it for your setup?
>>
>> According to my profiler, I think it's using 30% of the CPU time during
>> my custom org-clock-sum just to get ITEM.  I suppose it's because it
>> thinks it has to grab and cache everything when all I'm after is ITEM.
>
> Not sure here. Getting ITEM is just a single regexp match.
> May you share the profiler report? (M-x profiler-report-write-profile)

I'm experiencing some bugs with `profiler-find-profile'.  Might be
related to all the type changes that happened recently in Emacs master
(I like to run on the bleeding edge so I can experience as much pain and
frustration as possible).

Anyways, it's not just a regexp match since `org-cached-entry-get' tells
`org-entry-properties' to get everything.  We can see here almost all of
the slowdown for me occurs because of `org-element-context' which is
called in the `find-ts' lambda that search's for TIMESTAMP and
TIMESTAMP_IA.

8780  33%- org-cached-entry-get
8780  33% - org-entry-properties
8696  32%  - #
8236  31%   + org-element-context


The comment written just above `org-element-properties-map' says this:

#+BEGIN_SRC elisp
;; There is purposely no function like `org-element-properties' that
;; returns a list of properties.  Such function would tempt the users
;; to (1) run it, creating a whole new list; (2) filter over that list
;; - the process requiring a lot of extra consing, adding a load onto
;; Emacs GC, memory used, and slowing things up as creating new lists
;; is not free for CPU.
#+END_SRC

This implies that the function `org-entry-properties' is just a bad
idea.  Although giving it an argument for WHICH does make it better.

It seems like it's only used in about 4 places in our codebase so if we
wanted to go further, we could potentially see more performance
enhancements if we obsolete this function as well.

I haven't really looked into it much though so I apologize if my
analyses is wrong.  Which it definitely could be.



Info manual looks funny because of tables and footnotes

2024-05-08 Thread Morgan Smith
Hello!

I'm currently running org from commit
'773bba92a8e2b927427cf1daf374fa774fe834e0' (+ my own modifications).

I was reading the manual `8.4.2 The clock table' when I got down to the
`:block' section I noticed it was way too wide.  Like this wide:


‘:block’
 The time block to consider.  This block is specified either
 absolutely, or relative to the current time and may be any of these
 formats:

 ‘2007-12-31’   
 New year eve 2007
 ‘2007-12’  
 December 2007



That's too wide.  Looking at the source
[[file:doc/org-manual.org::7046]], it's pretty clear to see that it's
that wide because of the footnote that is inside the org table.

I think I'll end my investigation here and let you guys fix this one.

Thanks,

Morgan



[PATCH] Fix typos surrounding `org-agenda-skip-scheduled-repeats-after-deadline'

2024-05-24 Thread Morgan Smith
* etc/ORG-NEWS: Fix typos.
* testing/lisp/test-org-agenda.el: Fix typos.
---

Hello!

I was reading through the news and found this entry a little confusing.  Also
the variable names wheren't quite right.

Thanks,

Morgan

 etc/ORG-NEWS| 13 +
 testing/lisp/test-org-agenda.el | 26 +-
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 585b2b262..e2bbe3e0e 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -666,19 +666,16 @@ arrived.
 Users who often work with altering REPL prompts may consider reducing
 the default 5 second value of the new option.
 
-*** ~repeated-after-deadline~ value of 
~org-agenda-skip-scheduled-repeats-after-deadline~ is moved to a new 
customization
+*** ~repeated-after-deadline~ value of 
~org-agenda-skip-scheduled-if-deadline-is-shown~ is moved to a new customization
 
 A new custom option ~org-agenda-skip-scheduled-repeats-after-deadline~
 is introduced in place of ~repeated-after-deadline~ value of
-~org-agenda-skip-scheduled-repeats-after-deadline~.
+~org-agenda-skip-scheduled-if-deadline-is-shown~.
 
-Introducing a new option allow more control over agenda display and
-resolves a confusion about the meaning of ~repeated-after-deadline~.
-~repeated-after-deadline~ has nothing to do with /showing/ deadline.
-It just prevents agenda display repeated scheduled entries past
-deadline. The following example illustrates the meaning:
+The following example would no longer show in the agenda as scheduled
+after January 5th with the new customization set to ~t~.
 
-: * TODO Do me every day before Jan, 12th (included)
+: * TODO Do me every day until Jan, 5th (inclusive)
 : SCHEDULED: <2024-01-03 Wed +1d> DEADLINE: <2024-01-05 Fri>
 
 The old customization will continue to work, ensuring backwards compatibility.
diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index d4f7e71b4..72a005e71 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -598,7 +598,7 @@ DEADLINE: " (cdr timestamp
   (dolist (org-agenda-skip-scheduled-repeats-after-deadline '(nil t))
 (org-test-at-time "2024-01-01 8:00"
   (org-test-with-temp-text-in-file "
-* TODO Do me every day before Jan, 12th (included)
+* TODO Do me every day until Jan, 5th (inclusive)
 SCHEDULED: <2024-01-03 Wed +1d> DEADLINE: <2024-01-05 Fri>
 "
 (let ((org-agenda-span 'week)
@@ -614,15 +614,15 @@ SCHEDULED: <2024-01-03 Wed +1d> DEADLINE: <2024-01-05 Fri>
(string-match-p
 "Week-agenda (W01):
 Monday  1 January 2024 W01
-  [^:]+:In   4 d.:  TODO Do me every day before Jan, 12th (included)
+  [^:]+:In   4 d.:  TODO Do me every day until Jan, 5th (inclusive)
 Tuesday 2 January 2024
 Wednesday   3 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)
 Thursday4 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)
 Friday  5 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)
-  [^:]+:Deadline:   TODO Do me every day before Jan, 12th (included)
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)
+  [^:]+:Deadline:   TODO Do me every day until Jan, 5th (inclusive)
 Saturday6 January 2024
 Sunday  7 January 2024"
 (buffer-string)))
@@ -631,19 +631,19 @@ Sunday  7 January 2024"
  (string-match-p
   "Week-agenda (W01):
 Monday  1 January 2024 W01
-  [^:]+:In   4 d.:  TODO Do me every day before Jan, 12th (included)
+  [^:]+:In   4 d.:  TODO Do me every day until Jan, 5th (inclusive)
 Tuesday 2 January 2024
 Wednesday   3 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)
 Thursday4 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)
 Friday  5 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)
-  [^:]+:Deadline:   TODO Do me every day before Jan, 12th (included)
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)
+  [^:]+:Deadline:   TODO Do me every day until Jan, 5th (inclusive)
 Saturday6 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)
 Sunday  7 January 2024
-  [^:]+:Scheduled:  TODO Do me every day before Jan, 12th (included)"
+  [^:]+:Scheduled:  TODO Do me every day until Jan, 5th (inclusive)"
   (buffer-string))
   (org-test-agenda--kill-all-agendas
 
-- 
2.41.0




[PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines

2024-06-12 Thread Morgan Smith
With a file like this:

* headline 1
** headline 2

We currently produce an imenu tree that looks like this:

'(("headline 1" ("headline 2" . marker-2)))

imenu has no clue where "headline 1" is located and thus the user
can't navigate to it.  With this patch installed imenu knows where
non-leaf headlines are as the tree will now look like this:

'(("headline 1" . marker-1)
  ("headline 1" ("headline 2" . marker-2)))

Quirks:

With the default `imenu-flatten' value of nil, it is still impossible
to visit non-leaf headlines and no change is perceived.

Setting `imenu-flatten' to 'group works as expected with the quirk
that top level headlines don't end up in the group.

Ex:
* Headline 1
Group is "*"
Setting the group to "Headline 1" somehow might be nice but would
require upstream changes in imenu.
** Headline 2
Group is "Headline 1"
*** Headline 3
Group is "Headline 1:Headline 2"

Everything seems to work as expected when `imenu-flatten' is set to
'prefix or 'annotation.

* lisp/org-compat.el (org-imenu-get-tree): Add the current headline to
the tree as a simple item even if it isn't a leaf.
---
 lisp/org-compat.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index d6620f962..a1152186d 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -1447,8 +1447,8 @@ This also applied for speedbar access."
   (let* ((m (point-marker))
  (item (propertize headline 'org-imenu-marker m 'org-imenu t)))
 (push m org-imenu-markers)
-(if (>= level last-level)
-(push (cons item m) (aref subs level))
+ (push (cons item m) (aref subs level))
+ (unless (>= level last-level)
   (push (cons item
   (cl-mapcan #'identity (cl-subseq subs (1+ level
 (aref subs level))
-- 
2.45.1




Re: imenu vs. refile goto (was Re: [PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines)

2024-06-14 Thread Morgan Smith
Samuel Wales  writes:

> how does imenu compare against completion systems like ido combined with org
> refile set to goto?  when would you use imenu vs. other completion?

For my specific use case, all I want is a simple minibuffer completion
that offers me all the headings in the current buffer and allows me to
jump to the one I select.  Something which sounds easy, but in practice
I haven't found a good solution yet.

I assume you are suggesting `C-u M-x org-refile'.  This is a solution I
haven't seen before.  Being the refile command doesn't make it an
intuitive solution and I wonder if users would be able to find this
easily (I wasn't able to).

In order to get completion for subheadings I had to add this to my
configuration file:
`(setopt org-refile-targets '((nil . (:maxlevel . 5'

I'm actually quite pleased with this interface and it is really nice.
It is exactly what I'm trying to get from imenu.

My only gripe is that it is org specific whereas imenu isn't.  If I get
imenu working the way I like, then I'll benefit in other modes as well.



Re: [PATCH] lisp/org-compat.el: Allow using imenu to visit non-leaf headlines

2024-06-14 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> Quirks:
>>
>> With the default `imenu-flatten' value of nil, it is still impossible
>> to visit non-leaf headlines and no change is perceived.
>
> But no regressions, right? Especially in older Emacs versions with
> `imenu-flatten' not yet available.
>

No regressions that I can tell from my testing.  Both emacs 29.3 (before
imenu-flatten) and emacs-master show no change whatsoever (from what I
can tell) when imenu-flatten is nil.

>> Setting `imenu-flatten' to 'group works as expected with the quirk
>> that top level headlines don't end up in the group.
>
> We may add a top-level group, can't we?
>

This does not seem to be a feature.  You can confirm this yourself by
reading the comment in `imenu--flatten-index-alist' that says "PREFIX is
for internal use only".

> We may also consider changing the default value of `imenu-flatten' in
> Org buffers to non-nil.

In my opinion this is not a good idea.  While the UI of imenu is better
when `imenu-flatten' is set, that's not a decision we should make for
users.

The inability for imenu to visit non-leaf nodes with the default UI is
probably something that should be fixed in emacs core.

> But what would be the best default?

IMO that would be 'prefix.  Which is equivalent to setting it to `t'.



[PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2024-06-15 Thread Morgan Smith
* lisp/org.el (org-tags-sort-hierarchy): New function.
(org-tags-sort-function): Add new function to type.
* testing/lisp/test-org.el (test-org/tags-sort-hierarchy): New test
---

This is one of those things that I thought would be easy but then ended up
hard.

I wrote this so that items in my agenda would sort nicely.  Items tagged in the
same hierarchy would end up next to each other.

 lisp/org.el  | 38 +-
 testing/lisp/test-org.el | 19 +++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index 750b060f3..b828f4127 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2955,7 +2955,8 @@ is better to limit inheritance to certain tags using the 
variables
  (const :tag "No sorting" nil)
  (const :tag "Alphabetical" org-string<)
  (const :tag "Reverse alphabetical" org-string>)
- (function :tag "Custom function" nil)))
+  (const :tag "Hierarchy" org-tags-sort-hierarchy)
+  (function :tag "Custom function" nil)))
 
 (defvar org-tags-history nil
   "History of minibuffer reads for tags.")
@@ -4262,6 +4263,41 @@ See `org-tag-alist' for their structure."
   ;; Preserve order of ALIST1.
   (append (nreverse to-add) alist2)
 
+(defun org-tags-sort-hierarchy (tag1 tag2)
+  "Sort tags TAG1 and TAG2 by the tag hierarchy.
+Sorting is done alphabetically.  This function is intended to be a value
+of `org-tags-sort-function'."
+  (let ((sort-func #'org-string<)
+(group-alist (or org-tag-groups-alist-for-agenda
+ org-tag-groups-alist)))
+(if (not (and org-group-tags
+  group-alist))
+(funcall sort-func tag1 tag2)
+  (let* ((tag-path-function
+  ;; Returns a list of tags describing the tag path
+  ;; ex: '("top level tag" "second level" "tag")
+  (lambda (tag)
+(let ((result (list tag)))
+  (while (setq tag
+   (map-some
+(lambda (key tags)
+  (when (and (member tag tags)
+ ;; infinite loop (only catches 
the trivial case)
+ (not (string-equal tag key)))
+key))
+group-alist))
+(push tag result))
+  result)))
+ (tag1-path (funcall tag-path-function tag1))
+ (tag2-path (funcall tag-path-function tag2)))
+;; value< was added in Emacs 30
+;; (value< tag1-path tag2-path)
+(catch :result
+  (dotimes (n (min (length tag1-path) (length tag2-path)))
+(unless (string-equal (nth n tag1-path) (nth n tag2-path))
+  (throw :result (funcall sort-func (nth n tag1-path) (nth n 
tag2-path)
+  (< (length tag1-path) (length tag2-path)))
+
 (defun org-priority-to-value (s)
   "Convert priority string S to its numeric value."
   (or (save-match-data
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index f21e52bfd..59b16a62a 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -8508,6 +8508,25 @@ Paragraph"
(org-mode-restart)
(let ((org-tag-alist-for-agenda nil)) (org-tags-expand "{A+}"))
 
+(ert-deftest test-org/tags-sort-hierarchy ()
+  "Test `org-tags-sort-hierarchy' specifications."
+  (let ((org-tag-groups-alist-for-agenda
+ '(("A" "B" "D" "z" "zz")
+   ("B" "y")
+   ("C" "x")
+   ("D" "w")
+   ("E" "C" "v")))
+(test-list '("v" "w" "x" "y" "zz" "z" "E" "D" "C" "B" "A")))
+(should (equal
+ '("A" "B" "y" "D" "w" "z" "zz" "E" "C" "x" "v")
+ (sort test-list #'org-tags-sort-hierarchy
+  ;; infinite loop (tag "A" should not be in the "A" group)
+  (let ((org-tag-groups-alist-for-agenda
+ '(("A" "A" "B")))
+(test-list '("B" "A")))
+(should (equal
+ '("A" "B")
+ (sort test-list #'org-tags-sort-hierarchy)
 
 ;;; TODO keywords
 
-- 
2.45.1




[PATCH] Testing: Add tests for `org-tags-sort-function'

2024-06-15 Thread Morgan Smith
* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): New
test.
---

Hello!

I just recently sent in a patch to add a new possible value for
`org-tags-sort-function'.  To ease discussion there, I thought I'd add a nice
little test here so I can later show off the feature I'm trying to add.

Do notice that TODO I put in the comment though.  Not sure what the solution to
that should be.

It would be nice to accept this patch even with the TODO so I can make patches
in the other discussion that extend this test.  Or I suppose I could try being
patient.  That doesn't sound as fun though.

Thanks,

Morgan

 testing/lisp/test-org-agenda.el | 49 +
 1 file changed, 49 insertions(+)

diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index 778f91e8e..c1092df3b 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -655,6 +655,55 @@ Sunday  7 January 2024
   (buffer-string))
   (org-test-agenda--kill-all-agendas
 
+(ert-deftest test-org-agenda/tags-sorting ()
+  "Test if `org-agenda' sorts tags according to `org-tags-sort-function'."
+  (let ((org-agenda-custom-commands
+ '(("f" "no fluff" todo ""
+((org-agenda-todo-keyword-format "")
+ (org-agenda-overriding-header "")
+ (org-agenda-prefix-format "")
+ (org-agenda-remove-tags t)
+ (org-agenda-sorting-strategy '(tag-up))
+;; Sorting doesn't care about `org-tag-alist'.  This is only
+;; here for later when we add sorting methods that do
+(org-tag-alist
+ '((:startgrouptag)
+   ("group_a")
+   (:grouptags)
+   ("tag_a_1")
+   ("tag_a_2")
+   (:endgrouptag)
+   (:startgroup)
+   ("tag_b_1")
+   ("tag_b_2")
+   (:endgroup)
+   ("groupless")
+   ("lonely"
+(org-test-agenda-with-agenda
+ (string-join
+  '("* TODO group_a :group_a:"
+"* TODO tag_a_1 :tag_a_1:"
+"* TODO tag_a_2 :tag_a_2:"
+"* TODO tag_b_1 :tag_b_1:"
+"* TODO tag_b_2 :tag_b_2:"
+"* TODO groupless :groupless:"
+"* TODO lonely :lonely:")
+  "\n")
+ (dolist (org-tags-sort-function '(nil org-string< org-string>))
+   (should
+(string-equal
+ (progn
+   (org-agenda nil "f")
+   (substring-no-properties (buffer-string)))
+ (pcase org-tags-sort-function
+   ;; TODO: a value of `nil' sorts it!  That's not what the
+   ;; customize menu of `org-tags-sort-function' says!  It
+   ;; says "No sorting".
+   ((or 'nil 'org-string<)
+"group_a\ngroupless\nlonely\ntag_a_1\ntag_a_2\ntag_b_1\ntag_b_2\n")
+   ('org-string>
+
"tag_b_2\ntag_b_1\ntag_a_2\ntag_a_1\nlonely\ngroupless\ngroup_a\n"
+
 (ert-deftest test-org-agenda/goto-date ()
   "Test `org-agenda-goto-date'."
   (unwind-protect
-- 
2.45.1




Re: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx

2024-06-19 Thread Morgan Smith
Ihor Radchenko  writes:

> Ping ;)

So I gave up on this specific patch because I wrote a patch to just
rewrite the entire `org-clock-sum' function using org-element API.
Attached is the `org-clock-sum' rewrite patch which I've been using for
a while with no issues.  I have half finished patches locally to add
more clocktable tests and to add clocktable benchmarks which is why I
hadn't submitted this yet.

This probably belongs in this email thread instead:
https://list.orgmode.org/87y18vxgjs.fsf@localhost/

I believe I fixed all the points you brought up in that thread.


While this patch is probably ready to merge, doing so might cause
regressions to the fix applied in commit
fd8ddf2874ca00505aa096c6172ea750cd5e9eaa.

Ideally the fix in that commit should be ported to the org-element API.
Notably, the malformed clock from the email thread from that commit is
parsed a little strangely by org-element.  I'm not sure what effect this
has on my rewrite patch but regardless, we should probably fix this.
Notice how ":day-end" and ":minute-end" are set but not ":hour-start" or
":minute-start".

I have attached a rough patch adding a test for this case but my brain
is currently melting from a heatwave so it might take me a while to make
it into something good.  Feel free to work on this yourself if you have
time.

"CLOCK: [2012-01-01 sun. 00rr:01]--[2012-01-01 sun. 00:02] =>  0:01"

(clock
 (:standard-properties ...
  :status closed :value
  (timestamp
   (:standard-properties ...
:type inactive-range :range-type daterange :raw-value
"[2012-01-01 sun. 00rr:01]--[2012-01-01 sun. 00:02]" :year-start 2012
:month-start 1 :day-start 1 :hour-start nil :minute-start nil :year-end
2012 :month-end 1 :day-end 1 :hour-end 0 :minute-end 2))
  :duration "0:01"))

>From d407b357ba4285acc3c2548e38f034b5665b40bb Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Thu, 11 Apr 2024 12:23:21 -0400
Subject: [PATCH 1/2] lisp/org-clock.el (org-clock-sum): Rewrite using element
 api

---
 lisp/org-clock.el | 200 +++---
 1 file changed, 99 insertions(+), 101 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index c6fd507b0..5842c1cc7 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -33,15 +33,13 @@
 
 (require 'cl-lib)
 (require 'org)
+(require 'org-element)
 
 (declare-function calendar-iso-to-absolute "cal-iso" (date))
 (declare-function notifications-notify "notifications" (&rest params))
 (declare-function org-element-property "org-element-ast" (property node))
-(declare-function org-element-contents-end "org-element" (node))
-(declare-function org-element-end "org-element" (node))
 (declare-function org-element-type "org-element-ast" (node &optional anonymous))
 (declare-function org-element-type-p "org-element-ast" (node types))
-(defvar org-element-use-cache)
 (declare-function org-inlinetask-at-task-p "org-inlinetask" ())
 (declare-function org-inlinetask-goto-beginning "org-inlinetask" ())
 (declare-function org-inlinetask-goto-end "org-inlinetask" ())
@@ -2021,105 +2019,68 @@ TSTART and TEND can mark a time range to be considered.
 HEADLINE-FILTER is a zero-arg function that, if specified, is called for
 each headline in the time range with point at the headline.  Headlines for
 which HEADLINE-FILTER returns nil are excluded from the clock summation.
-PROPNAME lets you set a custom text property instead of :org-clock-minutes."
+PROPNAME lets you set a custom text property instead of :org-clock-minutes.
+
+Clocking entries that are open (as in don't have an end time) that are
+not the current clocking entry will be ignored."
   (with-silent-modifications
-(let* ((re (concat "^\\(\\*+\\)[ \t]\\|^[ \t]*"
-		   org-clock-string
-		   "[ \t]*\\(?:\\(\\[.*?\\]\\)-+\\(\\[.*?\\]\\)\\|=>[ \t]+\\([0-9]+\\):\\([0-9]+\\)\\)"))
-	   (lmax 30)
-	   (ltimes (make-vector lmax 0))
-	   (level 0)
-	   (tstart (cond ((stringp tstart) (org-time-string-to-seconds tstart))
-			 ((consp tstart) (float-time tstart))
-			 (t tstart)))
-	   (tend (cond ((stringp tend) (org-time-string-to-seconds tend))
-		   ((consp tend) (float-time tend))
-		   (t tend)))
-	   (t1 0)
-	   time)
-  (remove-text-properties (point-min) (point-max)
-			  `(,(or propname :org-clock-minutes) t
-:org-clock-force-headline-inclusion t))
-  (save-excursion
-	(goto-char (point-max))
-	(while (re-search-backward re nil t)
-  (let* ((element (save-match-data (org-element-at-point)))
- (element-type (org-element-type element)))
-	(cond
-	 ((and (eq element-type 'clock) (match-end 2))
-	  ;; Two time stamps.
-  (condition-case ni

[PATCH] ; fix typos

2024-06-19 Thread Morgan Smith
* doc/org-manual.org: Add missing '~'.
* lisp/org-element-ast.el (org-element-deferred): Fix typo in docstring.
* lisp/org-element.el (org-element-archive-tag,
org-element-drawer-re-nogroup, org-element--cache-log-message): Fix
typos in docstring.
(org-element--current-element, org-element--cache-find,
org-element-cache-map): Fix typos in comments.
---
 doc/org-manual.org  |  2 +-
 lisp/org-element-ast.el |  2 +-
 lisp/org-element.el | 13 ++---
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 96c90eaab..9bc58de60 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -23190,7 +23190,7 @@ * Footnotes
 =(diary-date 12 1 2005)= or =(diary-date 1 12 2005)= or =(diary-date
 2005 12 1)=, depending on the settings.  This has been the source of
 much confusion.  Org mode users can resort to special versions of
-these functions, namely ~org-date~, ~org-anniversary~, ~org-cyclic, and
+these functions, namely ~org-date~, ~org-anniversary~, ~org-cyclic~, and
 ~org-block~.  These work just like the corresponding ~diary-~
 functions, but with stable ISO order of arguments (year, month, day)
 wherever applicable, independent of the value of
diff --git a/lisp/org-element-ast.el b/lisp/org-element-ast.el
index fba6b37e6..2c767e1ad 100644
--- a/lisp/org-element-ast.el
+++ b/lisp/org-element-ast.el
@@ -270,7 +270,7 @@ Return value is the containing property name, as a keyword, 
or nil."
   "Dynamically computed value.
 
 The value can be obtained by calling FUNCTION with containing syntax
-node as first argument and ARGS list as remainting arguments.
+node as first argument and ARGS list as remaining arguments.
 
 If the function throws `:org-element-deferred-retry' signal, assume
 that the syntax node has been modified by side effect and retry
diff --git a/lisp/org-element.el b/lisp/org-element.el
index 191bb5698..811d32276 100644
--- a/lisp/org-element.el
+++ b/lisp/org-element.el
@@ -110,7 +110,7 @@
 ;; to current setup.
 
 (defconst org-element-archive-tag "ARCHIVE"
-  "Tag marking a substree as archived.")
+  "Tag marking a subtree as archived.")
 
 (defconst org-element-citation-key-re
   (rx "@" (group (one-or-more (any word "-.:?!`'/*@+|(){}<>&_^$#%~"
@@ -181,8 +181,7 @@ Drawer's name is located in match group 1.")
   (rx line-start (0+ (any ?\s ?\t))
   ":" (1+ (any ?- ?_ word)) ":"
   (0+ (any ?\s ?\t)) line-end)
-  "Regexp matching opening or closing line of a drawer.
-Drawer's name is located in match group 1.")
+  "Regexp matching opening or closing line of a drawer.")
 
 (defconst org-element-dynamic-block-open-re
   (rx line-start (0+ (any ?\s ?\t))
@@ -4679,7 +4678,7 @@ element it has to parse."
;;
;; In general, the checks below should be as efficient as
;; possible, especially early in the `cond' form.  (The
-   ;; early checks will contribute to al subsequent parsers as
+   ;; early checks will contribute to all subsequent parsers as
;; well).
(cond
;; Item.
@@ -5956,7 +5955,7 @@ better to remove the commands advised in such a way from 
this list.")
 
 (defmacro org-element--cache-log-message (format-string &rest args)
   "Add a new log message for org-element-cache.
-FORMAT-STRING and ARGS are the same arguments as in `foramt'."
+FORMAT-STRING and ARGS are the same arguments as in `format'."
   `(when (or org-element--cache-diagnostics
  (eq org-element--cache-self-verify 'backtrace))
  (let* ((format-string (concat (format "org-element-cache diagnostics(%s): 
"
@@ -6226,7 +6225,7 @@ the cache."
;; children starting at the same pos.
(not (org-element-type-p hashed '(section org-data table
   hashed
-;; No appriate HASHED.  Search the cache.
+;; No appropriate HASHED.  Search the cache.
 (while node
   (let* ((element (avl-tree--node-data node))
 (begin (org-element-begin element)))
@@ -8323,7 +8322,7 @@ the cache."
 limit-count))
 (cache-walk-abort))
   ;; Make sure that we have a cached
-  ;; element at the new STAR.
+  ;; element at the new START.
   (when start (element-match-at-point)))
 ;; Check if the buffer or cache has been modified.
 (unless (org-with-base-buffer nil
-- 
2.45.1




Re: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx

2024-06-19 Thread Morgan Smith
Ihor Radchenko  writes:

>> Ideally the fix in that commit should be ported to the org-element API.
>> Notably, the malformed clock from the email thread from that commit is
>> parsed a little strangely by org-element.  I'm not sure what effect this
>> has on my rewrite patch but regardless, we should probably fix this.
>> Notice how ":day-end" and ":minute-end" are set but not ":hour-start" or
>> ":minute-start".
>
> That's expected.
> We have the following _syntax_ description for clock lines:
>
> https://orgmode.org/worg/org-syntax.html#Clocks...
> clock: INACTIVE-TIMESTAMP-RANGE DURATION
>
> And [2012-01-01 sun. 00rr:01] is a perfectly valid timestamp without
> time part. Org does allow arbitrary additional text in the timestamps.
> (it is by design, to allow future extensions of the syntax, like the
> planned timezone support)

My specific issue is that the ":*-end" stuff can be set when the
"*-start" stuff is not.

Running the following snippet on the following file results in this:

=== snippet
(org-element-cache-map
 (lambda (clock)
   "Calculate end-time - start-time"
   (let ((timestamp (org-element-property :value clock)))
 (- (float-time (org-timestamp-to-time timestamp t))
(float-time (org-timestamp-to-time timestamp)
 :granularity 'element
 :restrict-elements 'clock)



 file
* Test
CLOCK: [2023-11-15 Wed 15:26]--[2023-11-15 Wed 16:12] =>  0:46
calculated time: 2760.0
CLOCK: [2023-11-15 Wed 15rr:26]--[2023-11-15 Wed 16:12] =>  0:46
calculated time: 58320.0
CLOCK: [2023-11-15 Wed 15:26]--[2023-11-15 Wed 16rr:12] =>  0:46
calculated time: 0.0
CLOCK: [2023-11-15 Wed 15rr:26]--[2023-11-15 Wed 16rr:12] =>  0:46
calculated time: 0.0
===

=== results
(2760.0 58320.0 0.0 0.0)
==

What this shows is that an invalid end will cause the entry to be
ignored, but an invalid start will not.

We can totally claim that this is a feature, not a bug, if you would
like.  As this essentially treats
"CLOCK: [2023-11-15 Wed 15rr:26]--[2023-11-15 Wed 16:12] =>  0:46"
as
"CLOCK: [2023-11-15 Wed]--[2023-11-15 Wed 16:12] => 16:12"


If this is a feature then my previous patch is actually ready to merge.



Parse malformed clocklines (was: Re: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx)

2024-06-24 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>>> That's expected.
>>> We have the following _syntax_ description for clock lines:
>>>
>>> https://orgmode.org/worg/org-syntax.html#Clocks...>> clock: 
>>> INACTIVE-TIMESTAMP-RANGE DURATION
>> ...
>> My specific issue is that the ":*-end" stuff can be set when the
>> "*-start" stuff is not.
>> ...
>> CLOCK: [2023-11-15 Wed 15rr:26]--[2023-11-15 Wed 16:12] =>  0:46
>> calculated time: 58320.0
>> ...
>> What this shows is that an invalid end will cause the entry to be
>> ignored, but an invalid start will not.
>
> Yup. Everything makes sense, but only within syntax spec. Actual code
> that handles such clock lines is another story.
>
> Warning is important because a number of people use clock functionality
> for billing - miscalculating clock sums can literally affect people's
> income :)
>
> I think that the best course of action when a problematic timestamp
> without opening/closing time is encountered is:
>
> 1. Warn user
> 2. Still calculate the duration, assuming 0s in time (simply because
>previous versions of Org mode did it)
>
> (2) is kind of debatable, but I can imagine some users might make use of
>
> such feature by putting clocks by hand:
> CLOCK: [2023-11-15 Wed]--[2023-11-16 Thu] => 24:00
>
> These users may then simply suppress the warning.

I'm very tempted to just make it hard rule that clocklines need to be
fully specified.  If you take a look at the attached patch, it shows one
way we could do this.

First, I modify the timestamp parser so the ":*-end" variables don't
always inherit the ":*-start" variables.  They can be nil independent of
the start.

Then in the clockline parser I ensure that every single field is set.
If it isn't, then I set the timestamp to nil so it won't be used.

This preserves the flexibility you want in timestamp parsing while also
catching many malformed clocklines.

Let me know what you think of this approach.

Also a couple questions about this approach:

1. Will changing the timestamp parser have far-reaching implications?
Is this something I should avoid?

2. Is there a way to give user errors in the parser code?  I'm using
`org-element--cache-warn' in my patch but I'm not sure that's the right
thing to do.  (if it is the right thing then we need to define it
earlier in the file)


>From 82231fb4b11b780488c66b4e5f0ee6fcf6643f1d Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 19 Jun 2024 13:41:50 -0400
Subject: [PATCH] Warn about invalid clock lines

---
 lisp/org-element.el | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/lisp/org-element.el b/lisp/org-element.el
index d6ad9824a..113cd6059 100644
--- a/lisp/org-element.el
+++ b/lisp/org-element.el
@@ -2312,6 +2312,25 @@ Return a new syntax node of `clock' type containing `:status',
 			 (unless (bolp) (skip-chars-forward " \t"))
 			 (count-lines before-blank (point
 	   (end (point)))
+  (when (and value ;; Clock lines don't need timestamps
+ (or
+  (not
+   (and (org-element-property :year-start value)
+(org-element-property :month-start value)
+(org-element-property :day-start value)
+(org-element-property :hour-start value)
+(org-element-property :minute-start value)))
+  (and (eq status 'closed)
+   (not (and (org-element-property :year-end value)
+ (org-element-property :month-end value)
+ (org-element-property :day-end value)
+ (org-element-property :hour-end value)
+ (org-element-property :minute-end value))
+(setq value nil)
+(org-element--cache-warn "Invalid clock element at %s:%d: \"%s\""
+ (buffer-name)
+ (line-number-at-pos begin t)
+ (buffer-substring-no-properties begin end)))
   (org-element-create
'clock
(list :status status
@@ -4393,15 +4412,14 @@ Assume point is at the beginning of the timestamp."
 		  hour-start (nth 2 date)
 		  minute-start (nth 1 date
 	;; Compute date-end.  It can be provided directly in timestamp,
-	;; or extracted from time range.  Otherwise, it defaults to the
-	;; same values as date-start.
+	;; or extracted from time range.
 	(unless diaryp
 	  (let ((date (and date-end (org-parse-time-string date-end t
-	(setq year-end (or (nth 5 date) year-start)
-		  month-end (o

Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2024-08-28 Thread Morgan Smith
Ihor Radchenko  writes:

> Ihor Radchenko  writes:
>
>> ... may you please elaborate what kind of sorting
>> order does your function imply? ...
>
> It has been over a month. Have you had a chance to look into my
> questions?

After you asked that question, I decided to demonstrate the changes I
wanted to make by also submitting a test.  However, I thought it prudent
to first test what we already have.  So I was waiting for a response to
my other patch (linked below) adding that test before continuing with
this thread.

I didn't make my intentions clear.  Sorry about that.  Thanks for
following up!  I appreciate that.

https://list.orgmode.org/ch3pr84mb342464f6458f91ee5800545ac5...@ch3pr84mb3424.namprd84.prod.outlook.com/



Re: [PATCH] Testing: Add tests for `org-tags-sort-function'

2024-09-02 Thread Morgan Smith
I've attached an updated patch.

This one has a test where we actually don't sort tags by using the
`ignore' function.

Ihor Radchenko  writes:

>> +;; Sorting doesn't care about `org-tag-alist'.  This is only
>> +;; here for later when we add sorting methods that do
>> 
>
> Then, please refrain from setting alist in this patch and add it later,
> when testing the relevant methods.

Done.  Now my tag names don't really make sense but I suppose that's not
a big issue.

>> +(org-test-agenda-with-agenda
>> + (string-join
>> +  '("* TODO group_a :group_a:"
>> +"* TODO tag_a_1 :tag_a_1:"
>> +"* TODO tag_a_2 :tag_a_2:"
>> +"* TODO tag_b_1 :tag_b_1:"
>> +"* TODO tag_b_2 :tag_b_2:"
>> +"* TODO groupless :groupless:"
>> +"* TODO lonely :lonely:")
>> +  "\n")
>> + (dolist (org-tags-sort-function '(nil org-string< org-string>))
>> +   (should
>> +(string-equal
>> + (progn
>> +   (org-agenda nil "f")
>> +   (substring-no-properties (buffer-string)))
>> + (pcase org-tags-sort-function
>> +   ;; TODO: a value of `nil' sorts it!  That's not what the
>> +   ;; customize menu of `org-tags-sort-function' says!  It
>> +   ;; says "No sorting".
>
> Right. When sort function is not set agenda specifically (but not other
> users of org-tags-sort-function) falls back to alphabetical sorting.

As far as I can tell the only place where it doesn't fall back is in
`org-set-tags'.

> In fact, the docstring does not at all mention that
> `org-tags-sort-function' is honored at all:
>
>   (defcustom org-agenda-sorting-strategy ...
> ...
> tag-up Sort alphabetically by last tag, A-Z.
> tag-down   Sort alphabetically by last tag, Z-A.
>
> We need to fix this docstring documenting `org-tags-sort-function', I think.

I don't understand what is happening in `org-entries-lessp' or how it
even manages to use `org-agenda-sorting-strategy' so I'm going to
refrain from trying to document things I don't understand.

>> +   ((or 'nil 'org-string<)
>> +
>> "group_a\ngroupless\nlonely\ntag_a_1\ntag_a_2\ntag_b_1\ntag_b_2\n")
>> +   ('org-string>
>> +
>> "tag_b_2\ntag_b_1\ntag_a_2\ntag_a_1\nlonely\ngroupless\ngroup_a\n"
>
> Nitpick: it would be more readable to use `string-join' here as well.

Done


>From 04fe8d44aba77b79a3a0f6ad332a0fb07bdb21a2 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Sat, 15 Jun 2024 11:27:34 -0400
Subject: [PATCH] Testing: Add tests for `org-tags-sort-function'

* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): New
test.
---
 testing/lisp/test-org-agenda.el | 44 +
 1 file changed, 44 insertions(+)

diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index 778f91e8e..a06d02064 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -655,6 +655,50 @@ Sunday  7 January 2024
   (buffer-string))
   (org-test-agenda--kill-all-agendas
 
+(ert-deftest test-org-agenda/tags-sorting ()
+  "Test if `org-agenda' sorts tags according to `org-tags-sort-function'."
+  (let ((org-agenda-custom-commands
+ '(("f" "no fluff" todo ""
+((org-agenda-todo-keyword-format "")
+ (org-agenda-overriding-header "")
+ (org-agenda-prefix-format "")
+ (org-agenda-remove-tags t)
+ (org-agenda-sorting-strategy '(tag-up)))
+(org-test-agenda-with-agenda
+ (string-join
+  '("* TODO group_a :group_a:"
+"* TODO tag_a_1 :tag_a_1:"
+"* TODO tag_a_2 :tag_a_2:"
+"* TODO tag_b_1 :tag_b_1:"
+"* TODO tag_b_2 :tag_b_2:"
+"* TODO groupless :groupless:"
+"* TODO lonely :lonely:")
+  "\n")
+ (dolist (org-tags-sort-function '(nil org-string< org-string> ignore))
+   (should
+(string-equal
+ (string-trim
+  (progn
+(org-agenda nil "f")
+(substring-no-properties (buffer-string
+ (pcase org-tags-sort-function
+   ;; Not sorted
+   ('ignore
+(string-join
+ '("group_a" "tag_a_1" "tag_a_2" "tag_b_1" "tag_b_2" "groupless" "lonely")
+ "\n"))
+   ;; TODO: a value of `nil' sorts it!  That's not what the
+   ;; customize menu of `org-tags-sort-function' says!  It
+   ;; says "No sorting".
+   ((or 'nil 'org-string< 'always)
+(string-join
+ '("group_a" "groupless" "lonely" "tag_a_1" "tag_a_2" "tag_b_1" "tag_b_2")
+ "\n"))
+   ('org-string>
+(string-join
+ '("tag_b_2" "tag_b_1" "tag_a_2" "tag_a_1" "lonely" "groupless" "group_a")
+ "\n")
+
 (ert-deftest test-org-agenda/goto-date ()
   "Test `org-agenda-goto-date'."
   (unwind-protect
-- 
2.45.2



[PATCH] Remove Emacs<24.4 compatibility code

2024-12-10 Thread Morgan Smith
* lisp/org-goto.el: Don't conditionally define keys based on if
`isearch-other-control-char' is bound.

Org mode only supports Emacs 27 and later.
---
 lisp/org-goto.el | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/lisp/org-goto.el b/lisp/org-goto.el
index f75cc9ed8..aa2a9328a 100644
--- a/lisp/org-goto.el
+++ b/lisp/org-goto.el
@@ -134,14 +134,9 @@ When nil, you can use these keybindings to navigate the 
buffer:
  (org-defkey map "\C-c\C-u" 'outline-up-heading)
  map)))
 
-;; `isearch-other-control-char' was removed in Emacs 24.4.
-(if (fboundp 'isearch-other-control-char)
-(progn
-  (define-key org-goto-local-auto-isearch-map "\C-i" 
'isearch-other-control-char)
-  (define-key org-goto-local-auto-isearch-map "\C-m" 
'isearch-other-control-char))
-  (define-key org-goto-local-auto-isearch-map "\C-i" nil)
-  (define-key org-goto-local-auto-isearch-map "\C-m" nil)
-  (define-key org-goto-local-auto-isearch-map [return] nil))
+(define-key org-goto-local-auto-isearch-map "\C-i" nil)
+(define-key org-goto-local-auto-isearch-map "\C-m" nil)
+(define-key org-goto-local-auto-isearch-map [return] nil)
 
 (defun org-goto--local-search-headings (string bound noerror)
   "Search and make sure that any matches are in headlines."
-- 
2.46.0




Re: [PATCH] Testing: Add tests for `org-tags-sort-function'

2024-12-10 Thread Morgan Smith
Hello!

Morgan Smith  writes:

> Ihor Radchenko  writes:
>>> +   ;; TODO: a value of `nil' sorts it!  That's not what the
>>> +   ;; defcustom :type of `org-tags-sort-function' says!  It
>>> +   ;; says "No sorting".
>>
>> Right. When sort function is not set agenda specifically (but not other
>> users of org-tags-sort-function) falls back to alphabetical sorting.
>
> As far as I can tell the only place where it doesn't fall back is in
> `org-set-tags'.
>
>> In fact, the docstring does not at all mention that
>> `org-tags-sort-function' is honored at all:
>>
>>   (defcustom org-agenda-sorting-strategy ...
>> ...
>> tag-up Sort alphabetically by last tag, A-Z.
>> tag-down   Sort alphabetically by last tag, Z-A.
>>
>> We need to fix this docstring documenting `org-tags-sort-function', I think.
>
> I don't understand what is happening in `org-entries-lessp' or how it
> even manages to use `org-agenda-sorting-strategy' so I'm going to
> refrain from trying to document things I don't understand.

I've polished the patch slightly but it's almost identical to what I
submitted before.  There is one remaining issue as you can see above but
I'm trying my best to ignore it.  I believe this patch can be applied
before the issue is fixed.  I would like this patch to be accepted so I
can build off of it to submit another patch that includes an alternative
function to set `org-tags-sort-function' to.

>From e0ec4bf01230b3e38276daf709dce9e459f1b3e0 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Sat, 15 Jun 2024 11:27:34 -0400
Subject: [PATCH] Testing: Add tests for `org-tags-sort-function'

* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): New
test.
---
 testing/lisp/test-org-agenda.el | 44 +
 1 file changed, 44 insertions(+)

diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index 778f91e8e..5f9916232 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -655,6 +655,50 @@ Sunday  7 January 2024
   (buffer-string))
   (org-test-agenda--kill-all-agendas
 
+(ert-deftest test-org-agenda/tags-sorting ()
+  "Test if `org-agenda' sorts tags according to `org-tags-sort-function'."
+  (let ((org-agenda-custom-commands
+ '(("f" "no fluff" todo ""
+((org-agenda-todo-keyword-format "")
+ (org-agenda-overriding-header "")
+ (org-agenda-prefix-format "")
+ (org-agenda-remove-tags t)
+ (org-agenda-sorting-strategy '(tag-up)))
+(org-test-agenda-with-agenda
+ (string-join
+  '("* TODO group_a :group_a:"
+"* TODO tag_a_1 :tag_a_1:"
+"* TODO tag_a_2 :tag_a_2:"
+"* TODO tag_b_1 :tag_b_1:"
+"* TODO tag_b_2 :tag_b_2:"
+"* TODO groupless :groupless:"
+"* TODO lonely :lonely:")
+  "\n")
+ (dolist (org-tags-sort-function '(nil org-string< org-string> ignore))
+   (should
+(string-equal
+ (string-trim
+  (progn
+(org-agenda nil "f")
+(substring-no-properties (buffer-string
+ (pcase org-tags-sort-function
+   ;; Not sorted
+   ('ignore
+(string-join
+ '("group_a" "tag_a_1" "tag_a_2" "tag_b_1" "tag_b_2" "groupless" "lonely")
+ "\n"))
+   ;; TODO: a value of `nil' sorts it!  That's not what the
+   ;; defcustom :type of `org-tags-sort-function' says!  It
+   ;; says "No sorting".
+   ((or 'nil 'org-string<)
+(string-join
+ '("group_a" "groupless" "lonely" "tag_a_1" "tag_a_2" "tag_b_1" "tag_b_2")
+ "\n"))
+   ('org-string>
+(string-join
+ '("tag_b_2" "tag_b_1" "tag_a_2" "tag_a_1" "lonely" "groupless" "group_a")
+ "\n")
+
 (ert-deftest test-org-agenda/goto-date ()
   "Test `org-agenda-goto-date'."
   (unwind-protect
-- 
2.46.0



Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2024-12-24 Thread Morgan Smith
Hello!

I have updated the patch with many improvements and tests.

Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> * lisp/org.el (org-tags-sort-hierarchy): New function.
>> (org-tags-sort-function): Add new function to type.
>> * testing/lisp/test-org.el (test-org/tags-sort-hierarchy): New test
>> ---
>>
>> This is one of those things that I thought would be easy but then ended up
>> hard.
>>
>> I wrote this so that items in my agenda would sort nicely.  Items tagged in 
>> the
>> same hierarchy would end up next to each other.
>
>> +  "Sort tags TAG1 and TAG2 by the tag hierarchy.
>> +Sorting is done alphabetically.  This function is intended to be a value
>> +of `org-tags-sort-function'."
>
> Thanks for the patch, but may you please elaborate what kind of sorting
> order does your function imply? In particular, I am wondering about the
> ordering of tags from different groups?

Hopefully the test case I added shows how the sorting work.  The tags
end up sorted as this:

("group_a" "tag_a_1" "tag_a_2" "groupless" "lonely" "tag_b_1" "tag_b_2")

Things end up with other things of their group.  Things higher in the
heirarchy end up earlier.  When things are on the same level (like
tag_a_1 and tag_a_2 or group_a and groupless) they are sorted using
`org-sort-function'.

> Also, what happens when there
> are no tag groups defined? (These questions should be answered in the
> docstring and the etc/ORG-NEWS entry you need to add, announcing the new
> feature)

Fallback to `org-sort-function'.

> Also, sorting may not be alphabetical, depending on the value of
> `org-sort-function'.

I have corrected this in the documentation.

>From 3f25374bbfd4134cea6ce0708633d500e8b41a89 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Fri, 14 Jun 2024 17:38:41 -0400
Subject: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

* lisp/org.el (org-tags-sort-hierarchy): New function.
(org-tags-sort-function): Add new function to type.
* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): Test
sorting with a value of 'org-tags-sort-hierarchy.
* etc/ORG-NEWS: Announce the new feature.
---
 etc/ORG-NEWS|  7 ++
 lisp/org.el | 41 -
 testing/lisp/test-org-agenda.el | 32 -
 3 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 4c41f981c..62e8bb4ca 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -196,6 +196,13 @@ English.  The default value is ~t~ as the CSL standard assumes that
 English titles are specified in sentence-case but the bibtex
 bibliography format requires them to be written in title-case.
 
+*** New tags sorting function ~org-tags-sort-hierarchy~
+
+By setting ~org-tags-sort-function~ to ~org-tags-sort-hierarchy~, tags
+are sorted taking their hierarchy into account.  See ~org-tag-alist~
+for how to set up a tag hierarchy.  Secondary sorting is done using
+~org-sort-function~.
+
 ** New functions and changes in function arguments
 
 # This also includes changes in function behavior from Elisp perspective.
diff --git a/lisp/org.el b/lisp/org.el
index 748f258a2..6f5bf066d 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -231,6 +231,7 @@ Stars are put in group 1 and the trimmed body in group 2.")
 (defvar org-element--timestamp-regexp)
 (defvar org-indent-indentation-per-level)
 (defvar org-radio-target-regexp)
+(defvar org-sort-function)
 (defvar org-target-link-regexp)
 (defvar org-target-regexp)
 (defvar org-id-overriding-file-name)
@@ -2966,7 +2967,8 @@ default."
 	  (const :tag "Default sorting" nil)
 	  (const :tag "Alphabetical" org-string<)
 	  (const :tag "Reverse alphabetical" org-string>)
-	  (function :tag "Custom function" nil)))
+  (const :tag "Sort by hierarchy" org-tags-sort-hierarchy)
+  (function :tag "Custom function" nil)))
 
 (defvar org-tags-history nil
   "History of minibuffer reads for tags.")
@@ -4275,6 +4277,43 @@ See `org-tag-alist' for their structure."
   ;; Preserve order of ALIST1.
   (append (nreverse to-add) alist2)
 
+(defun org-tags-sort-hierarchy (tag1 tag2)
+  "Sort tags TAG1 and TAG2 by the tag hierarchy.
+See `org-tag-alist' for how to set up a tag hierarchy.  Secondary
+sorting is done using `org-sort-function'.  This function is intended to
+be a value of `org-tags-sort-function'."
+  (let ((group-alist (or org-tag-groups-alist-for-agenda
+ org-tag-groups-alist)))
+(if (not (and org-group-tags
+  group-alist))
+(funcall org-sort-function tag1 tag2)
+  (l

Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2025-04-21 Thread Morgan Smith
Ihor Radchenko  writes:

> Ihor Radchenko  writes:
>
>>> Fallback to `org-sort-function'.
>>
>> I am not sure if it is the best idea to hard-code using
>> `org-sort-function' here.
>>
>> What if a user wants specific `org-sort-function' for, say, table
>> sorting, but something else for tags?
>> Secondary sorting would better be customizeable.
>>
>> Maybe you can have a function generator that will work like
>>
>> (org-tags-sort-hierarchy-by org-sort-function)
>> or
>> (org-tags-sort-hierarchy-by #'string<)
>
> Morgan, may I know if you are still interested to finish the patch?

Yes I am very interested in finishing this patch.  Has it been 10 months
since I started this already?  Oh how time flys!

I hit a roadblock trying to figure out how best to implement your
suggestion.

After running something like this:

(setopt org-tags-sort-function (org-tags-sort-hierarchy-by
org-sort-function))

The help buffer for `org-tags-sort-function' displays a lambda as the
value which isn't very helpful.  It also isn't conducive to users using
the customize interface (I think?  I'm not going to pretend I actually
use that thing).

I wanted to set up a default named function that used org-sort-function.
I was playing around with defalias to generate that function and copy
the docstring but I think that was very hacky and I can't seem to get it
to work with the latest Emacs.

At that point it got put on my backburner and it hasn't been worked on
since :P

Looking into it now it looks like an oclosure would be an interesting
way to implement this.  However, I don't think those got backported to
older emacsen so I guess that's out.

Maybe I'm thinking too hard about this but it feels to me like it should
be a solved problem.  How would you do this?



Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2025-07-12 Thread Morgan Smith
My apologies.  The previous email reverted too much of my `org-set-tags'
changes.  These changes should work now.  Please see attached.
Commentary from previous email is still applicable. 


>From 7ea6b1bbdf7e3df075f419269c3ebbd5b2578359 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 27 May 2025 15:14:34 -0400
Subject: [PATCH 1/2] Allow `org-tags-sort-function' to be a list of functions

* lisp/org.el (org-tags-sort-function): Describe new feature.  Add
'(repeat function) to the type.
(org-tags-sort): New function
* lisp/org-agenda.el (org-cmp-tag): Use `org-tags-sort'.
* lisp/org-mouse.el (org-mouse-tag-menu, org-mouse-popup-global-menu):
Use `org-tags-sort'.
* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): Test
new functionality.
* etc/ORG-NEWS: Announce the new feature.
---
 etc/ORG-NEWS|  6 ++
 lisp/org-agenda.el  |  4 ++--
 lisp/org-mouse.el   |  8 
 lisp/org.el | 35 +
 testing/lisp/test-org-agenda.el | 32 ++
 5 files changed, 67 insertions(+), 18 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 62502a678..1f29415ee 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -349,6 +349,12 @@ behaviour of other exporters. In this case, to exclude a section from
 the table of contents, mark it as =:UNNUMBERED: notoc= in its
 properties.
 
+*** ~org-tags-sort-function~ can now be a list of functions
+
+~org-tags-sort-function~ can now be set to a list of functions.
+Subsequent sorting functions will be used if two tags are found to be
+equivalent.  See docstring for more information.
+
 *** New option ~org-cite-basic-complete-key-crm-separator~
 
 This option makes ~org-cite~'s ~basic~ insert processor use
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 7f0a6ee75..a10ae1888 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7570,8 +7570,8 @@ org-cmp-tag
 (cond ((not (or ta tb)) nil)
 	  ((not ta) +1)
 	  ((not tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) ta tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) tb ta) +1
+	  ((org-tags-sort ta tb) -1)
+	  ((org-tags-sort tb ta) +1
 
 (defsubst org-cmp-time (a b)
   "Compare the time-of-day values of strings A and B."
diff --git a/lisp/org-mouse.el b/lisp/org-mouse.el
index a282f004c..bc0857d3c 100644
--- a/lisp/org-mouse.el
+++ b/lisp/org-mouse.el
@@ -427,13 +427,13 @@ org-mouse-tag-menu
(let ((tags (org-get-tags nil t)))
  (org-mouse-keyword-menu
   (sort (mapcar #'car (org-get-buffer-tags))
-(or org-tags-sort-function #'org-string<))
+#'org-tags-sort)
   (lambda (tag)
 	(org-mouse-set-tags
 	 (sort (if (member tag tags)
 		   (delete tag tags)
 		 (cons tag tags))
-	   (or org-tags-sort-function #'org-string<
+   #'org-tags-sort)))
   (lambda (tag) (member tag tags))
   ))
'("--"
@@ -504,7 +504,7 @@ org-mouse-popup-global-menu
  ("Check Tags"
   ,@(org-mouse-keyword-menu
 	 (sort (mapcar #'car (org-get-buffer-tags))
-   (or org-tags-sort-function #'org-string<))
+   #'org-tags-sort)
  (lambda (tag) (org-tags-sparse-tree nil tag)))
   "--"
   ["Custom Tag ..." org-tags-sparse-tree t])
@@ -515,7 +515,7 @@ org-mouse-popup-global-menu
  ("Display Tags"
   ,@(org-mouse-keyword-menu
 	 (sort (mapcar #'car (org-get-buffer-tags))
-   (or org-tags-sort-function #'org-string<))
+   #'org-tags-sort)
  (lambda (tag) (org-tags-view nil tag)))
   "--"
   ["Custom Tag ..." org-tags-view t])
diff --git a/lisp/org.el b/lisp/org.el
index 0a406d7cc..ae32baee5 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -3009,13 +3009,21 @@ org-tags-sort-function
   "When set, tags are sorted using this function as a comparator.
 When the value is nil, use default sorting order.  The default sorting
 is alphabetical, except in `org-set-tags' where no sorting is done by
-default."
+default.
+
+This can also be a list of functions.  If a function returns nil when
+sorting two tags regardless of order (to indicate both tags are equal),
+then the next function in the list will be called to determine sort
+order.  The functions are allowed to call `org-tag-sort' which will skip
+to the next sort function in the list.  If the last function in the list
+calls `org-tag-sort' then it will use the default sorting order."
   :group 'org-tags
   :type '(choice
 	  (const :tag "Default sorting" nil)
 	  (const :tag "Alphabetical" org-string<)
 	  (const :tag "Reverse alphabetical" org-string>)
-	  (function :tag "Custom function&qu

Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2025-07-12 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> --- a/lisp/org.el
>> +++ b/lisp/org.el
>> @@ -3015,7 +3015,8 @@ org-tags-sort-function
>>(const :tag "Default sorting" nil)
>>(const :tag "Alphabetical" org-string<)
>>(const :tag "Reverse alphabetical" org-string>)
>> -  (function :tag "Custom function" nil)))
>> +  (function :tag "Custom function" nil)
>> +  (repeat function)))
>
> We also need to describe how the new multi-function sorting is working.
> In particular, we should document that function can call `org-tags-sort'
> recursively and that it will yield sorting using functions specified
> _after_ current function in `org-tags-sort-function'.
> There is also somewhat unexpected behavior when a given sort function is
> the last in the list - it will implicitly call `org-tags-sort' with
> org-tags-sort-function = nil, which might be somewhat surprising (unless
> clearly warned about).
>

I added all the documentation into the docstring of org-tags-sort-function.

>> +(defun org-tags-sort (tag1 tag2)
>> +  "Sort tags TAG1 and TAG2 according to the value of 
>> `org-tags-sort-function'."
>> +  (let ((org-tags-sort-function
>> + (cond ((functionp org-tags-sort-function) (list 
>> org-tags-sort-function))
>> +   ((consp org-tags-sort-function) org-tags-sort-function)
>> +   ((null  org-tags-sort-function) (list #'org-string<)
>> +(catch :org-tags-sort-return
>> +  (dolist (sort-fun org-tags-sort-function)
>> +;; So the function can call `org-tags-sort'
>> +(let ((org-tags-sort-function (cdr org-tags-sort-function)))
>> +  (cond
>> +   ((funcall sort-fun tag1 tag2) ; tag1 < tag2
>> +(throw :org-tags-sort-return t))
>> +   ((funcall sort-fun tag2 tag1) ; tag1 > tag2
>> +(throw :org-tags-sort-return nil))
>> +   (t ; tag1 = tag2
>> +'continue-loop)))
>
> What happens when tags are completely equal, according to every function
> in `org-tags-sort-function'?
>

Then no sorting would occur.  In the test suite we test for this by using the
'ignore function.

>> --- a/testing/lisp/test-org.el
>> +++ b/testing/lisp/test-org.el
>> @@ -8484,7 +8484,7 @@ test-org/toggle-tag
>>;; Special case: Handle properly tag inheritance.  In particular, do
>>;; not set inherited tags.
>>(should
>> -   (equal "* H1 :tag:\n** H2 :tag2:tag:"
>> +   (equal "* H1 :tag:\n** H2 :tag:tag2:"
>>(org-test-with-temp-text "* H1 :tag:\n** H2 :tag2:"
>>  (let ((org-use-tag-inheritance t)
>>(org-tags-column 1))
>
> Why did you have to change this test?

I did not understand the intent of the conditional in `org-set-tags' and
changed behavior I shouldn't have.  Sorry about that.  All fixed now.

Please see attached patches.

The only changes I've made from last time is adding documentation to the
docstring and fixing the `org-set-tags' stuff I messed up. (+ minor edits to
commit message, news entry, and added another comment in `org-tags-sort').

>From 8e82972b3b69c8510769e2a33f4c4742376235be Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 27 May 2025 15:14:34 -0400
Subject: [PATCH 1/2] Allow `org-tags-sort-function' to be a list of functions

* lisp/org.el (org-tags-sort-function): Describe new feature.  Add
'(repeat function) to the type.
(org-tags-sort): New function
* lisp/org-agenda.el (org-cmp-tag): Use `org-tags-sort'.
* lisp/org-mouse.el (org-mouse-tag-menu, org-mouse-popup-global-menu):
Use `org-tags-sort'.
* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): Test
new functionality.
* etc/ORG-NEWS: Announce the new feature.
---
 etc/ORG-NEWS|  6 ++
 lisp/org-agenda.el  |  4 ++--
 lisp/org-mouse.el   |  8 
 lisp/org.el | 33 ++---
 testing/lisp/test-org-agenda.el | 32 
 5 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 62502a678..1f29415ee 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -349,6 +349,12 @@ behaviour of other exporters. In this case, to exclude a section from
 the table of contents, mark it as =:UNNUMBERED: notoc= in its
 properties.
 
+*** ~org-tags-sort-function~ can now be a list of functions
+
+~org-tags-sort-function~ can now be set to a list of functions.
+Subsequent sorting functions will be used if two tags

Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2025-06-21 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> However, I believe I have managed to come up with a flexible solution that 
>> does
>> involve allowing `org-tags-sort-function' to be a list of functions.  The 
>> trick
>> is to lexically bind the list each time so we don't get infinite recursion.
>> ...
>> +(defun org-tags-sort (tag1 tag2)
>> +  "Sort tags TAG1 and TAG2 according to the value of 
>> `org-tags-sort-function'."
>> +  (cond
>> ...
>> +   ((consp org-tags-sort-function)
>> +(let* ((sort-fun (car org-tags-sort-function))
>> +   ;; So the functions can call `org-tags-sort'
>> +   (org-tags-sort-function (cdr org-tags-sort-function)))
>> +  (funcall sort-fun tag1 tag2)))
>
> This implies that every possible sort function will take care about
> calling `org-tags-sort' recursively. I do not think that it is a good
> idea. Consider (setq org-tags-sort-function '(length< 
> org-tags-sort-hierarchy)).
> There is no way `length<' know to call `org-tags-sort'! So,
> `org-tags-sort-hierarchy' in the list will always be ignored. Not
> expected, IMHO.

Thank you for bring this case to my attention in such a concrete way.  I
have added fixes and tests to make this work.

>
> Instead, we can do the following:
>
> (catch :org-tags-sort-return ; also allow individual sort functions to exit 
> early
> (dolist (sort-fun org-tags-sort-function)
>   (cond
>((funcall sort-fun tag1 tag2) ; tag1 < tag2
>   (throw :org-tags-sort-return t))
>((funcall sort-fun tag2 tag1) ; tag1 > tag2
>   (throw :org-tags-sort-return nil))
>(t ; tag1 = tag2
>   'continue-loop)))
> ;; tag1 = tag2 for each function in the list
> nil)
>
> Then, if you need to perform custom sorting, you can return early from
> `org-tags-sort-hierarchy':
>
> (let ((org-tags-sort-function (delete #'org-tags-sort-hierarchy 
> org-tags-sort-function)))
>  (throw :org-tags-sort-return (org-tags-sort (nth n tag1-path) (nth n 
> tag2-path)))
>
> WDYT?

I'm not a fan of non-local exits while re-binding variable values.  Also
the `delete' and `throw' would cause errors in the case that
'org-tags-sort-function' is a function, not a list.

After adding your suggested changes and playing around with things I
think I have come up with a very nice solution.  Please see attached.

>From 4369436b05214486d9f7c29fdbc87aed991f18da Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 27 May 2025 15:14:34 -0400
Subject: [PATCH 1/2] Allow `org-tags-sort-function' to be a list of functions

* lisp/org.el (org-tags-sort-function): Add '(repeat function) to the
type.
(org-tags-sort): New function
* lisp/org-agenda.el (org-cmp-tag): Use `org-tags-sort'.
* lisp/org-mouse.el (org-mouse-tag-menu, org-mouse-popup-global-menu):
Use `org-tags-sort'.
* testing/lisp/test-org.el (test-org/toggle-tag): Fix tag order.
* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): Test
new functionality.
* etc/ORG-NEWS: Announce the new feature.
---
 etc/ORG-NEWS|  6 ++
 lisp/org-agenda.el  |  4 ++--
 lisp/org-mouse.el   |  8 
 lisp/org.el | 24 +---
 testing/lisp/test-org-agenda.el | 32 
 testing/lisp/test-org.el|  2 +-
 6 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 62502a678..810a0b02b 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -349,6 +349,12 @@ behaviour of other exporters. In this case, to exclude a section from
 the table of contents, mark it as =:UNNUMBERED: notoc= in its
 properties.
 
+*** ~org-tags-sort-function~ can now be a list of functions
+
+~org-tags-sort-function~ can now be set to a list of functions.
+Subsequent sorting functions will be used if two tags are found to be
+equivalent.
+
 *** New option ~org-cite-basic-complete-key-crm-separator~
 
 This option makes ~org-cite~'s ~basic~ insert processor use
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 7f0a6ee75..a10ae1888 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7570,8 +7570,8 @@ org-cmp-tag
 (cond ((not (or ta tb)) nil)
 	  ((not ta) +1)
 	  ((not tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) ta tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) tb ta) +1
+	  ((org-tags-sort ta tb) -1)
+	  ((org-tags-sort tb ta) +1
 
 (defsubst org-cmp-time (a b)
   "Compare the time-of-day values of strings A and B."
diff --git a/lisp/org-mouse.el b/lisp/org-mouse.el
index a282f004c..bc0857d3c 100644
--- a/lisp/org-mouse.el
+++ 

Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2025-08-05 Thread Morgan Smith
Ihor Radchenko  writes:

>> +This can also be a list of functions.  If a function returns nil when
>> +sorting two tags regardless of order (to indicate both tags are equal),
>
> Normally, comparison functions in Elisp return t when first argument
> (tag) is less than second, and nil otherwise (when >=). So, your
> description is rather confusing.
>

I updated the docstring.  Please let me know if it looks better.  I'm
not the greatest at documentation.

>> +*** New tags sorting function ~org-tags-sort-hierarchy~
>> +
>> +By setting ~org-tags-sort-function~ to ~org-tags-sort-hierarchy~, tags
>> +are sorted taking their hierarchy into account.  See ~org-tag-alist~
>> +for how to set up a tag hierarchy.
>
> We should better refer to 6.3 Tag Hierarchy section in the manual.
>

Done

>> +(defun org-tags-sort-hierarchy (tag1 tag2)
>> +  "Sort tags TAG1 and TAG2 by the tag hierarchy.
>> +See `org-tag-alist' for how to set up a tag hierarchy.  This function is
>> +intended to be a value of `org-tags-sort-function'."
>
> Same here.
>

Done

>> +;; value< was added in Emacs 30 and does not allow us to use
>> +;; `org-tags-sort-function'.
>> +;; (value< tag1-path tag2-path)
>
> Please mark it FIXME, so that we can use value< after the next Emacs
> release.

I deleted this comment as it's not useful.  `value<' cannot be used as
it doesn't support using custom sort functions.  The idea with the
comment was to let the reader know the following code is sort of
equivalent to `value<'.

>From 18cc15ff4be9af2c0d12242cd8ba1db8cf6d4ee8 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 27 May 2025 15:14:34 -0400
Subject: [PATCH 1/2] Allow `org-tags-sort-function' to be a list of functions

* lisp/org.el (org-tags-sort-function): Describe new feature.  Add
'(repeat function) to the type.
(org-tags-sort): New function
* lisp/org-agenda.el (org-cmp-tag): Use `org-tags-sort'.
* lisp/org-mouse.el (org-mouse-tag-menu, org-mouse-popup-global-menu):
Use `org-tags-sort'.
* testing/lisp/test-org-agenda.el (test-org-agenda/tags-sorting): Test
new functionality.
* etc/ORG-NEWS: Announce the new feature.
---
 etc/ORG-NEWS|  6 ++
 lisp/org-agenda.el  |  4 ++--
 lisp/org-mouse.el   |  8 +++
 lisp/org.el | 38 +
 testing/lisp/test-org-agenda.el | 32 ---
 5 files changed, 70 insertions(+), 18 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 375a0c424..a7763ef51 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -358,6 +358,12 @@ behaviour of other exporters. In this case, to exclude a section from
 the table of contents, mark it as =:UNNUMBERED: notoc= in its
 properties.
 
+*** ~org-tags-sort-function~ can now be a list of functions
+
+~org-tags-sort-function~ can now be set to a list of functions.
+Subsequent sorting functions will be used if two tags are found to be
+equivalent.  See docstring for more information.
+
 *** New option ~org-cite-basic-complete-key-crm-separator~
 
 This option makes ~org-cite~'s ~basic~ insert processor use
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 7f0a6ee75..a10ae1888 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7570,8 +7570,8 @@ org-cmp-tag
 (cond ((not (or ta tb)) nil)
 	  ((not ta) +1)
 	  ((not tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) ta tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) tb ta) +1
+	  ((org-tags-sort ta tb) -1)
+	  ((org-tags-sort tb ta) +1
 
 (defsubst org-cmp-time (a b)
   "Compare the time-of-day values of strings A and B."
diff --git a/lisp/org-mouse.el b/lisp/org-mouse.el
index a282f004c..bc0857d3c 100644
--- a/lisp/org-mouse.el
+++ b/lisp/org-mouse.el
@@ -427,13 +427,13 @@ org-mouse-tag-menu
(let ((tags (org-get-tags nil t)))
  (org-mouse-keyword-menu
   (sort (mapcar #'car (org-get-buffer-tags))
-(or org-tags-sort-function #'org-string<))
+#'org-tags-sort)
   (lambda (tag)
 	(org-mouse-set-tags
 	 (sort (if (member tag tags)
 		   (delete tag tags)
 		 (cons tag tags))
-	   (or org-tags-sort-function #'org-string<
+   #'org-tags-sort)))
   (lambda (tag) (member tag tags))
   ))
'("--"
@@ -504,7 +504,7 @@ org-mouse-popup-global-menu
  ("Check Tags"
   ,@(org-mouse-keyword-menu
 	 (sort (mapcar #'car (org-get-buffer-tags))
-   (or org-tags-sort-function #'org-string<))
+   #'org-tags-sort)
  (lambda (tag) (org-tags-sparse-tree nil tag)))
   "--"
   ["Custom Tag ..." org-tags-sparse-tre

Please remove .gitmodules file

2025-06-04 Thread Morgan Smith
Hello,

In commit ffb1652c4ada9595dc74773d3ccd409ec10280a1 Ihor removed the
"jump" submodule located at "testing/jump".  However, they did not
remove the ".gitmodules" file that specifies the git module.  This is
causing strange errors for me when using guix to build org-mode in its
container like environments.  Due to the complexity of the software
involved, it would be difficult to give a more precise error then what
I've described.

Please consider removing the file.

Thanks,

Morgan



Re: [PATCH] lisp/org.el: Add ability to sort tags by hierarchy

2025-05-30 Thread Morgan Smith
Ihor Radchenko  writes:
>
> Hmm. I think we can do it differently.
> Rather than trying to do secondary sorting in
> `org-tags-sort-hierarchy...' we can allow `org-tags-sort-function' to be
> a list of functions. Then, when sorting, if (sort-function a b) and
> (sort-function b a) return the same value (which means that a=b wrt a
> given sort-function), we try second sort function in the list, and so
> on.
>
> It will be (1) more universal; (2) follow our existing practice in
> `org-agenda-sorting-strategy'.
>
> WDYT?

Unless I am mistaken, performing the secondary sorting in the function is
mandatory.  I have included my reasoning below.

However, I believe I have managed to come up with a flexible solution that does
involve allowing `org-tags-sort-function' to be a list of functions.  The trick
is to lexically bind the list each time so we don't get infinite recursion.

Attached are two patches

* I don't believe we can remove secondary sorting from the comparison function

Just because a=b does not mean that they should end up beside each other.

Imagine a hierarchy like this:

 A
 | \
 B  D
 |
 F


So if we have B and F, we know B should be before F.

Now lets sort the list (B D F).

B and D don't have a hierarchy sorting so they fall back to alphabetical
secondary sorting.

The sorted list now looks like (B D).

We now compare D to F and again fall back to alphabetical sorting so the sorted
list looks like (B D F).

The type of sorting I'm trying to do would have the list end up as (B F D)
since F falls under B.

>From 7951ff4639197cd846e292bf30c731846e05c3f2 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Tue, 27 May 2025 15:14:34 -0400
Subject: [PATCH 1/2] Allow `org-tags-sort-function' to be a list of functions

* lisp/org.el (org-tags-sort-function): Add '(repeat function) to the
type.
(org-tags-sort): New function
* lisp/org-agenda.el (org-cmp-tag): Use `org-tags-sort'.
* lisp/org-mouse.el (org-mouse-tag-menu, org-mouse-popup-global-menu):
Use `org-tags-sort'.
* testing/lisp/test-org.el (test-org/toggle-tag): Fix tag order.
* etc/ORG-NEWS: Announce the new feature.
---
 etc/ORG-NEWS |  6 ++
 lisp/org-agenda.el   |  4 ++--
 lisp/org-mouse.el|  8 
 lisp/org.el  | 22 +++---
 testing/lisp/test-org.el |  2 +-
 5 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 62502a678..770abca2c 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -349,6 +349,12 @@ behaviour of other exporters. In this case, to exclude a section from
 the table of contents, mark it as =:UNNUMBERED: notoc= in its
 properties.
 
+*** ~org-tags-sort-function~ can now be a list of functions
+
+~org-tags-sort-function~ can now be set to a list of functions.  This
+allows sorting functions to call ~org-tags-sort~ recursively if they
+find two tags to be equivalent.
+
 *** New option ~org-cite-basic-complete-key-crm-separator~
 
 This option makes ~org-cite~'s ~basic~ insert processor use
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 7f0a6ee75..a10ae1888 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -7570,8 +7570,8 @@ The optional argument TYPE tells the agenda type."
 (cond ((not (or ta tb)) nil)
 	  ((not ta) +1)
 	  ((not tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) ta tb) -1)
-	  ((funcall (or org-tags-sort-function #'org-string<) tb ta) +1
+	  ((org-tags-sort ta tb) -1)
+	  ((org-tags-sort tb ta) +1
 
 (defsubst org-cmp-time (a b)
   "Compare the time-of-day values of strings A and B."
diff --git a/lisp/org-mouse.el b/lisp/org-mouse.el
index a282f004c..bc0857d3c 100644
--- a/lisp/org-mouse.el
+++ b/lisp/org-mouse.el
@@ -427,13 +427,13 @@ SCHEDULED: or DEADLINE: or ANYTHINGLIKETHIS:"
(let ((tags (org-get-tags nil t)))
  (org-mouse-keyword-menu
   (sort (mapcar #'car (org-get-buffer-tags))
-(or org-tags-sort-function #'org-string<))
+#'org-tags-sort)
   (lambda (tag)
 	(org-mouse-set-tags
 	 (sort (if (member tag tags)
 		   (delete tag tags)
 		 (cons tag tags))
-	   (or org-tags-sort-function #'org-string<
+   #'org-tags-sort)))
   (lambda (tag) (member tag tags))
   ))
'("--"
@@ -504,7 +504,7 @@ SCHEDULED: or DEADLINE: or ANYTHINGLIKETHIS:"
  ("Check Tags"
   ,@(org-mouse-keyword-menu
 	 (sort (mapcar #'car (org-get-buffer-tags))
-   (or org-tags-sort-function #'org-string<))
+   #'org-tags-sort)
  (lambda (tag) (org-tags-sparse-tree nil tag)))
   "--"
   ["Custom Tag ..." org-tags-sparse-tree t])
@@ -515,7 +515,7 @@ SCHEDULED: or DEADLINE: or ANYTHINGLIKETHIS:"
  ("Display Tags"
   ,@(org-

Re: [PATCH] Rewrite `org-clock-sum'

2025-08-10 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> I apologize for the delay in re-submitting this.  I have created a
>> number of tests to test the current implementation.  Then I have
>> reimplemented `org-clock-sum' using the org-element api (ensuring all
>> tests still pass).  Then I added a new feature that will issue a warning
>> if the timestamp is not fully specified.
>>
>> This brings a big performance improvement.
>
> Thanks!
> I have a few comments.
>
>> From d3d0174bb3bc9be108d50cca0e738db4d9f2598b Mon Sep 17 00:00:00 2001
>> From: Morgan Smith 
>> Date: Sat, 9 Aug 2025 14:09:20 -0400
>> Subject: [PATCH 4/6] Testing: test for malformed clock lines
>
> This test is failing unless patch #5 is applied. Something is off.
>

Somehow I got the impression that the existing error handling wasn't
doing anything about underspecified timestamps.  Not sure how I got that
impression because that test failing is proof that it is working.

I fixed everything up and merged my 5th and 6th patch together.  The
error handling also doesn't require condition-case anymore which is
nice.

>> +   (when (and org-clock-report-include-clocking-task
>> +  (eq (org-clocking-buffer) 
>> (current-buffer))
>> +  (eq (marker-position org-clock-hd-marker)
>> +  (org-element-begin 
>> headline-or-inlinetask))
>> +  (or (not tstart)
>> +  (>= (float-time org-clock-start-time) 
>> tstart))
>> +  (or (not tend)
>> +  (<= (float-time org-clock-start-time) 
>> tend)))
>
> This is not how it worked before. Previously, the clocked task was only
> included when tstart and tend are both non-nil. Could you please explain
> this change?
>

I've got a bad habit of wanting to change everything.  I've restored the
original conditionals.

This was actually why one of the inlinetask tests failed on the old
org-clock-sum implementation.  Now that test passes :).

>> +   ;; TODO: can inlinetasks contain inlinetasks?
>
> They cannot.

Thanks!

>From 3a008af13c25f3b87cc4cd7c440255c925a5c5b1 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 17 Apr 2024 17:51:35 -0400
Subject: [PATCH 1/5] Testing: Test clock times without timestamps

* testing/lisp/test-org-clock.el (test-org-clock/clocktable/insert):
Add a clock time that does not include timestamps.
---
 testing/lisp/test-org-clock.el | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 8a196ee96..62e4d7507 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -345,13 +345,12 @@ test-org-clock/clocktable/insert
(equal
 "| Headline | Time   |
 |--+|
-| *Total time* | *1:00* |
+| *Total time* | *2:00* |
 |--+|
-| H1   | 1:00   |"
+| H1   | 2:00   |"
 (org-test-with-temp-text "* H1\n"
-  (insert (org-test-clock-create-clock ". 1:00" ". 2:00"))
-
-  (goto-line 2)
+  (insert (org-test-clock-create-clock ". 1:00" ". 2:00")
+  "CLOCK: => 1:00\n")
   (require 'org-clock)
   (org-dynamic-block-insert-dblock "clocktable")
 
-- 
2.50.1

>From 41f228760a71900125496a85f9fd87d5155b2b0f Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 8 May 2024 10:36:07 -0400
Subject: [PATCH 2/5] Testing: New test test-org-clock/clocktable/open-clock

* testing/lisp/test-org-clock.el
(test-org-clock/clocktable/open-clock): New test.
---
 testing/lisp/test-org-clock.el | 33 +
 1 file changed, 33 insertions(+)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 62e4d7507..1ae91ebba 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -362,6 +362,39 @@ test-org-clock/clocktable/insert
 	 (point) (progn (search-forward "#+END:") (line-end-position 0
 	(delete-region (point) (search-forward "#+END:\n")))
 
+(ert-deftest test-org-clock/clocktable/open-clock ()
+  "Test open clocks.
+Open clocks should be ignored unless it is clocked in and
+`org-clock-report-include-clocking-task' is t."
+  (let ((time-reported "| Headline | Time   |
+|--+|
+| *Total time* | *1:00* |
+|--+|
+| H1   | 1:00   |")
+(time-not-reported "| Headline | Time   |
+|--+|
+| *Total time* | *0:00* |"))
+

[PATCH] Test files compile warnings

2025-08-13 Thread Morgan Smith
Hello,

I have decided to spend my time fixing compile time warnings in the test
files.  If you think that would be a waste of time, I'd have to agree.

I did find some minor things that might actually be important though.
The first 4 patches are legitimate improvements to the codebase in my
opinion.

The remaining 11 patches don't really do anything except silence
warnings.

Locally I have probably another 15 patches not included here to actually
get the number of warnings down to 0.  I figured I'd submit these first
before prettying those up.

Once we do have the number of warnings down to 0, then we might be able
to use the byte compiler to ensure our tests run as expected.  Ensuring
that return values are actually used could be a big help.

Thanks,

Morgan

>From 5cbe1a5f1718e9b9e12520cd8b91168baa99651a Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 23 Jul 2025 13:24:37 -0400
Subject: [PATCH 01/15] Testing: Add missing `should's

* testing/lisp/test-ob-scheme.el (test-ob-scheme/tables):
* testing/lisp/test-org-tempo.el (test-org-tempo/completion):
* testing/lisp/test-org.el (test-org/edit-headline):
Add a missing `should'.
---
 testing/lisp/test-ob-scheme.el | 9 +
 testing/lisp/test-org-tempo.el | 5 +++--
 testing/lisp/test-org.el   | 9 +
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/testing/lisp/test-ob-scheme.el b/testing/lisp/test-ob-scheme.el
index 79ce2b453..0fb79ad53 100644
--- a/testing/lisp/test-ob-scheme.el
+++ b/testing/lisp/test-ob-scheme.el
@@ -32,16 +32,17 @@
 
 (ert-deftest test-ob-scheme/tables ()
   "Test table output."
-  (equal "#+begin_src scheme
+  (should
+   (equal "#+begin_src scheme
 '(1 2 3)
 #+end_src
 
 #+RESULTS:
 | 1 | 2 | 3 |
 "
-	 (org-test-with-temp-text "#+begin_src scheme\n'(1 2 3)\n#+end_src"
-	   (org-babel-execute-maybe)
-	   (buffer-string
+	  (org-test-with-temp-text "#+begin_src scheme\n'(1 2 3)\n#+end_src"
+	(org-babel-execute-maybe)
+	(buffer-string)
 
 (ert-deftest test-ob-scheme/verbatim ()
   "Test verbatim output."
diff --git a/testing/lisp/test-org-tempo.el b/testing/lisp/test-org-tempo.el
index 7382b6dc4..800fef234 100644
--- a/testing/lisp/test-org-tempo.el
+++ b/testing/lisp/test-org-tempo.el
@@ -53,11 +53,12 @@ test-org-tempo/completion
 	(org-cycle)
 	(buffer-string
   ;; Tempo should not expand unknown snippets
-  (equal (org-test-with-temp-text ">From 7954ad095c34d44b8f1fd577e4ca2cc23dd99d9f Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 23 Jul 2025 14:11:53 -0400
Subject: [PATCH 02/15] * testing/lisp/test-org-attach-git.el: Remove git annex
 directory

* testing/lisp/test-org-attach-git.el
(test-org-attach-git/with-annex): Add missing unwindform to delete the
directory afterwards.
---
 testing/lisp/test-org-attach-git.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/testing/lisp/test-org-attach-git.el b/testing/lisp/test-org-attach-git.el
index e8b41fe8c..6b949afb5 100644
--- a/testing/lisp/test-org-attach-git.el
+++ b/testing/lisp/test-org-attach-git.el
@@ -37,7 +37,8 @@ test-org-attach-git/with-annex
  (shell-command "git config --global user.name \"John Doe\"")
 	 (shell-command "git init")
 	 (shell-command "git annex init")
-	 ,@body)
+	 ,@body))
+   (delete-directory tmpdir 'recursive
 
 (ert-deftest test-org-attach-git/use-annex ()
   (test-org-attach-git/with-annex
-- 
2.50.1

>From b0705f5485ee4584a86f8d89d24643ba14288981 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 23 Jul 2025 13:17:12 -0400
Subject: [PATCH 03/15] Change uses of deprecated `org-show-context(-detail)'

* doc/org-manual.org: Add `org-fold-show-context-detail' to vindex.
* doc/org-guide.org:
* lisp/ol.el:
* lisp/org-agenda.el:
* lisp/org-compat.el:
* lisp/org-fold.el:
* testing/lisp/test-org-fold.el:
* testing/lisp/test-org.el:
* testing/org-test.el:
Change all uses of `org-show-context' to `org-fold-show-context'.
Change all uses of `org-show-context-detail' to
`org-fold-show-context-detail'.
---
 doc/org-guide.org | 2 +-
 doc/org-manual.org| 5 +++--
 lisp/ol.el| 2 +-
 lisp/org-agenda.el| 2 +-
 lisp/org-compat.el| 2 +-
 lisp/org-fold.el  | 2 +-
 testing/lisp/test-org-fold.el | 4 ++--
 testing/lisp/test-org.el  | 4 ++--
 testing/org-test.el   | 8 
 9 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/doc/org-guide.org b/doc/org-guide.org
index cc7155c61..5e59fb025 100644
--- a/doc/org-guide.org
+++ b/doc/org-guide.org
@@ -2657,7 +2657,7 @@ * Footnotes
 [fn:2] If you do not want the line to be split, customize the variable
 ~org-M-RET-may-split-line~.
 
-[fn:3] See also the variable ~org-show-context-detail~ to decide how
+

Re: [PATCH] Rewrite `org-clock-sum'

2025-08-09 Thread Morgan Smith
Hello,

I apologize for the delay in re-submitting this.  I have created a
number of tests to test the current implementation.  Then I have
reimplemented `org-clock-sum' using the org-element api (ensuring all
tests still pass).  Then I added a new feature that will issue a warning
if the timestamp is not fully specified.

This brings a big performance improvement.

Thanks,

Morgan

>From 96e2c6dfbd597000ee90b91efe74d34cb7234c03 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 17 Apr 2024 17:51:35 -0400
Subject: [PATCH 1/6] Testing: Test clock times without timestamps

* testing/lisp/test-org-clock.el (test-org-clock/clocktable/insert):
Add a clock time that does not include timestamps.
---
 testing/lisp/test-org-clock.el | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 8a196ee96..62e4d7507 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -345,13 +345,12 @@ test-org-clock/clocktable/insert
(equal
 "| Headline | Time   |
 |--+|
-| *Total time* | *1:00* |
+| *Total time* | *2:00* |
 |--+|
-| H1   | 1:00   |"
+| H1   | 2:00   |"
 (org-test-with-temp-text "* H1\n"
-  (insert (org-test-clock-create-clock ". 1:00" ". 2:00"))
-
-  (goto-line 2)
+  (insert (org-test-clock-create-clock ". 1:00" ". 2:00")
+  "CLOCK: => 1:00\n")
   (require 'org-clock)
   (org-dynamic-block-insert-dblock "clocktable")
 
-- 
2.50.1

>From bee73f771bb7c3d3525583379404d734c53517a9 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 8 May 2024 10:36:07 -0400
Subject: [PATCH 2/6] Testing: New test test-org-clock/clocktable/open-clock

* testing/lisp/test-org-clock.el
(test-org-clock/clocktable/open-clock): New test.
---
 testing/lisp/test-org-clock.el | 33 +
 1 file changed, 33 insertions(+)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 62e4d7507..1ae91ebba 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -362,6 +362,39 @@ test-org-clock/clocktable/insert
 	 (point) (progn (search-forward "#+END:") (line-end-position 0
 	(delete-region (point) (search-forward "#+END:\n")))
 
+(ert-deftest test-org-clock/clocktable/open-clock ()
+  "Test open clocks.
+Open clocks should be ignored unless it is clocked in and
+`org-clock-report-include-clocking-task' is t."
+  (let ((time-reported "| Headline | Time   |
+|--+|
+| *Total time* | *1:00* |
+|--+|
+| H1   | 1:00   |")
+(time-not-reported "| Headline | Time   |
+|--+|
+| *Total time* | *0:00* |"))
+(dolist (org-clock-report-include-clocking-task '(nil t))
+  (dolist (actually-clock-in '(nil t))
+;; Without leading characters then `org-clock-hd-marker' doesn't
+;; get updated when clocktable is inserted and test fails.
+(org-test-with-temp-text "\n* H1\n"
+  (should
+   (equal
+(if (and org-clock-report-include-clocking-task
+ actually-clock-in)
+time-reported
+  time-not-reported)
+(progn
+  (if actually-clock-in
+  (org-clock-in nil (- (float-time) (* 60 60)))
+(goto-char (point-max))
+(insert (org-test-clock-create-clock "-1h")))
+  ;; Unless tstart and tend are fully specified it doesn't work
+  (test-org-clock-clocktable-contents ":tstart \"<-2d>\" :tend \"\""
+  (when actually-clock-in
+(org-clock-cancel)))
+
 (ert-deftest test-org-clock/clocktable/ranges ()
   "Test ranges in Clock table."
   ;; Relative time: Previous two days.
-- 
2.50.1

>From 36b6f4925f0cb2a647eabca8f1950f511d32841d Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Sat, 9 Aug 2025 14:04:07 -0400
Subject: [PATCH 3/6] Testing: Add tests for clocktables from inline tasks

* testing/lisp/test-org-clock.el
(test-org-clock/clocktable/inlinetask/insert)
(test-org-clock/clocktable/inlinetask/open-clock): New tests
---
 testing/lisp/test-org-clock.el | 75 ++
 1 file changed, 75 insertions(+)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 1ae91ebba..0058b9dc7 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -1497,5 +1497,80 @@ test-org-clock/special-range
cases
 (should-not failed)))
 
+;;; Inline tasks clocktable
+
+(require 'org-inlinetask)
+
+(ert-deftest test-org-clock/c

Re: [PATCH] Rewrite `org-clock-sum'

2025-08-31 Thread Morgan Smith
Hello,

I've been running this patch for quite a while now.  However, I just
realized it doesn't handle narrowing.  I have added a new patch in the
series that adds a test for when the :scope is "subtree" which my
previous patch failed.  The current iteration now has a
"org-with-wide-buffer" around the "org-element-lineage-map" which fixes
the issue.

I look forward to hearing your thoughts.

Thank you,

Morgan

>From 659dd156527db41d1012b3ba75ccb096fb61dc5a Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 17 Apr 2024 17:51:35 -0400
Subject: [PATCH 1/6] Testing: Test clock times without timestamps

* testing/lisp/test-org-clock.el (test-org-clock/clocktable/insert):
Add a clock time that does not include timestamps.
---
 testing/lisp/test-org-clock.el | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 8a196ee96..62e4d7507 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -345,13 +345,12 @@ test-org-clock/clocktable/insert
(equal
 "| Headline | Time   |
 |--+|
-| *Total time* | *1:00* |
+| *Total time* | *2:00* |
 |--+|
-| H1   | 1:00   |"
+| H1   | 2:00   |"
 (org-test-with-temp-text "* H1\n"
-  (insert (org-test-clock-create-clock ". 1:00" ". 2:00"))
-
-  (goto-line 2)
+  (insert (org-test-clock-create-clock ". 1:00" ". 2:00")
+  "CLOCK: => 1:00\n")
   (require 'org-clock)
   (org-dynamic-block-insert-dblock "clocktable")
 
-- 
2.51.0

>From c20d33a6dcd26e55e02e0a15d9ea9934eea785f9 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Wed, 8 May 2024 10:36:07 -0400
Subject: [PATCH 2/6] Testing: New test test-org-clock/clocktable/open-clock

* testing/lisp/test-org-clock.el
(test-org-clock/clocktable/open-clock): New test.
---
 testing/lisp/test-org-clock.el | 33 +
 1 file changed, 33 insertions(+)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 62e4d7507..1ae91ebba 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -362,6 +362,39 @@ test-org-clock/clocktable/insert
 	 (point) (progn (search-forward "#+END:") (line-end-position 0
 	(delete-region (point) (search-forward "#+END:\n")))
 
+(ert-deftest test-org-clock/clocktable/open-clock ()
+  "Test open clocks.
+Open clocks should be ignored unless it is clocked in and
+`org-clock-report-include-clocking-task' is t."
+  (let ((time-reported "| Headline | Time   |
+|--+|
+| *Total time* | *1:00* |
+|--+|
+| H1   | 1:00   |")
+(time-not-reported "| Headline | Time   |
+|--+|
+| *Total time* | *0:00* |"))
+(dolist (org-clock-report-include-clocking-task '(nil t))
+  (dolist (actually-clock-in '(nil t))
+;; Without leading characters then `org-clock-hd-marker' doesn't
+;; get updated when clocktable is inserted and test fails.
+(org-test-with-temp-text "\n* H1\n"
+  (should
+   (equal
+(if (and org-clock-report-include-clocking-task
+ actually-clock-in)
+time-reported
+  time-not-reported)
+(progn
+  (if actually-clock-in
+  (org-clock-in nil (- (float-time) (* 60 60)))
+(goto-char (point-max))
+(insert (org-test-clock-create-clock "-1h")))
+  ;; Unless tstart and tend are fully specified it doesn't work
+  (test-org-clock-clocktable-contents ":tstart \"<-2d>\" :tend \"\""
+  (when actually-clock-in
+(org-clock-cancel)))
+
 (ert-deftest test-org-clock/clocktable/ranges ()
   "Test ranges in Clock table."
   ;; Relative time: Previous two days.
-- 
2.51.0

>From 7c75691fec84bb485ec21f9aa52a29c20db55158 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Sat, 9 Aug 2025 14:04:07 -0400
Subject: [PATCH 3/6] Testing: Add tests for clocktables from inline tasks

* testing/lisp/test-org-clock.el
(test-org-clock/clocktable/inlinetask/insert)
(test-org-clock/clocktable/inlinetask/open-clock): New tests
---
 testing/lisp/test-org-clock.el | 73 ++
 1 file changed, 73 insertions(+)

diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index 1ae91ebba..f57c1e9d3 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -1497,5 +1497,78 @@ test-org-clock/special-range
cases
 (should-not failed)))

Re: BUG: Top row missing from org babel variable

2025-10-11 Thread Morgan Smith
patr...@plenihan.com writes:

> Org babel interprets the org table correctly as a variable in the first three
> cases, but if there is a middle rule and no top rule, the first row is 
> ignored.
> An example is shown below and in the attached bugs.org file.

It's not actually ignored.  In fact it is the only case of the 4 that
org correctly parses that the first line in the column names.  In your
first 3 examples org thinks that the column names are part of the data.

#+NAME: spending-3
|--+---+--+--+---|
| Date | Co-op | Starling | Cash | Notes |
|--+---+--+--+---|
| <2025-09-23 Tue> |   169 |   105.10 |   75 |   |

#+NAME: spending-4
| Date | Co-op | Starling | Cash | Notes |
|--+---+--+--+---|
| <2025-09-23 Tue> |   169 |   105.10 |   75 |   |


Notice how ":colname-names" is set for spending-4 but not spending-3.

#+begin_src elisp :results pp
(org-babel-process-params (list (cons :var "spending3=spending-3")))
#+end_src

#+RESULTS:
: ((:var spending3 hline ("Date" "Co-op" "Starling" "Cash" "Notes") hline
:("<2025-09-23 Tue>" 169 105.1 75 ""))
:  (:colname-names) (:rowname-names) (:result-params) (:result-type . value))

#+begin_src elisp :results pp
(org-babel-process-params (list (cons :var "spending4=spending-4")))
#+end_src

#+RESULTS:
: ((:var spending4 ("<2025-09-23 Tue>" 169 105.1 75 ""))
:  (:colname-names (spending4 "Date" "Co-op" "Starling" "Cash" "Notes"))
:  (:rowname-names) (:result-params) (:result-type . value))


Behavior can be controlled by setting ":colnames"


#+begin_src elisp :var spending4=spending-4
spending4
#+end_src

#+RESULTS:
| <2025-09-23 Tue> | 169 | 105.1 | 75 |   |

#+begin_src elisp :var spending4=spending-4 :colnames yes
spending4
#+end_src

#+RESULTS:
| Date | Co-op | Starling | Cash | Notes |
|--+---+--+--+---|
| <2025-09-23 Tue> |   169 |105.1 |   75 |   |

#+begin_src elisp :var spending4=spending-4 :colnames no
spending4
#+end_src

#+RESULTS:
| Date | Co-op | Starling | Cash | Notes |
| <2025-09-23 Tue> |   169 |105.1 |   75 |   |


You can check the manual for more information that even includes a
python example! [[info:org#Environment of a Code Block]]

I hope this helps!

Morgan



Re: [PATCH] Add tests for org-log-done

2025-10-06 Thread Morgan Smith
Ihor Radchenko  writes:

> Morgan Smith  writes:
>
>> Please enjoy these new tests!
>
> Thanks!
> PATCH 1/3 has unbalanced parenthesis. The situation is fixed in PATCH
> 2/3, but that's not ideal - we may run into problems if we try bisecting
> in the future. Please update the patches.
>
> Otherwise, LGTM.
>

Nice catch!  Time to start a checklist of things to do before submission
which includes test each invididual patch.  Do we have a checklist
somewhere?

If I remeber correctly John Wiegley posted on their blog
(https://newartisans.com/) something about experimenting with git commit
hooks to ensure tests are run at every stage.

If I remember correctly though he wasn't able to make it work for
rebasing which is how I made these patches :/.

>> I tested them on emacs 30.2 and emacs built from the development
>> branch at commit 9663c959c73 (2025-04-07).  I should probably start
>> running my tests on older emacs too...
>
> Note that we have automatic test setup in
> https://git.sr.ht/~bzg/org-mode-tests (also see https://sr.ht/~bzg/org/)
> They might be useful. Although I personally simply have multiple Emacs
> versions installed locally, because my OS makes it easy to install
> multiple versions of programs.

Oh nice!  Will look at that when I have a chance!

Thanks,

Morgan

>From a658f228c9089b6491bbee10806c063fb8b9b9d2 Mon Sep 17 00:00:00 2001
From: Morgan Smith 
Date: Sun, 5 Oct 2025 16:15:39 -0400
Subject: [PATCH 1/3] test-org/org-log-done: Refactor redundant code

* testing/lisp/test-org.el (test-org/org-log-done): Refactor redundant
code.  Expand docstring.  Add two TODOs.
---
 testing/lisp/test-org.el | 173 +--
 1 file changed, 59 insertions(+), 114 deletions(-)

diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 0dc7cb1a2..4481942ec 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -8899,131 +8899,76 @@ test-org/auto-repeat-maybe
 (buffer-string))
 
 (ert-deftest test-org/org-log-done ()
-  "Test `org-log-done' specifications."
-  ;; nil value.
-  (should
-   (string=
-"* DONE task"
-(let ((org-log-done nil)
-  (org-todo-keywords '((sequence "TODO" "DONE"
-  (org-test-with-temp-text
-  "* TODO task"
-(org-todo "DONE")
-(when (memq 'org-add-log-note (default-value 'post-command-hook))
-  (org-add-log-note))
-(buffer-string)
-  ;; `time' value.
-  (should
-   (string=
-(format
-"* DONE task
-CLOSED: %s"
-(org-test-with-temp-text ""
-  (org-insert-timestamp (current-time) t t)
-  (buffer-string)))
-(let ((org-log-done 'time)
-  (org-log-done-with-time t)
-  (org-todo-keywords '((sequence "TODO" "DONE"
-  (org-test-with-temp-text
-  "* TODO task"
-(org-todo "DONE")
-(when (memq 'org-add-log-note (default-value 'post-command-hook))
-  (org-add-log-note))
-(buffer-string)
-  (should
-   (string=
-(format
-"* DONE task
-CLOSED: %s"
-(org-test-with-temp-text ""
-  (org-insert-timestamp (current-time) nil t)
-  (buffer-string)))
-(let ((org-log-done 'time)
-  (org-log-done-with-time nil)
-  (org-todo-keywords '((sequence "TODO" "DONE"
-  (org-test-with-temp-text
-  "* TODO task"
-(org-todo "DONE")
-(when (memq 'org-add-log-note (default-value 'post-command-hook))
-  (org-add-log-note))
-(buffer-string)
-  ;; TODO: Test `note' value.
-  ;; Test startup overrides.
-  (should
-   (string=
-"#+STARTUP: nologdone
-* DONE task"
-(let ((org-log-done 'time)
-  (org-todo-keywords '((sequence "TODO" "DONE"
-  (org-test-with-temp-text
-  "#+STARTUP: nologdone
-* TODO task"
-(org-set-regexps-and-options)
-(org-todo "DONE")
-(when (memq 'org-add-log-note (default-value 'post-command-hook))
-  (org-add-log-note))
-(buffer-string)
-  (should
-   (string=
-(format
-"#+STARTUP: logdone
+  "Test `org-log-done' specifications.
+Behavior can be modified by setting `org-log-done', by keywords in
+\"#+STARTUP:\" or by special syntax in `org-todo-keywords'."
+  ;; TODO: Test special syntax in `org-todo-keywords'.
+  (let ((time-string
+ (org-test-with-temp-text ""
+   (org-insert-timestamp (current-time) t t)
+   (buffer-string)))
+(date-string
+ (org-test-with-temp-text ""
+   (or

Re: [PATCH] Add tests for org-log-done

2025-10-11 Thread Morgan Smith
Ihor Radchenko  writes:

> Applied, onto main.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=50de2f717
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8fceda647
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4611c2b7a

:D

> Multi-patch commits are not very common

The GNU Guix folk have trained me to make small commits.  Expect lots of
multi-patch commits from me :)

>> If I remeber correctly John Wiegley posted on their blog
>> (https://newartisans.com/) something about experimenting with git commit
>> hooks to ensure tests are run at every stage.
>
> In theory, such tests could be done on CI. But adding this kind of
> feature to CI tests will require work that may or may not be justified.
>
>> If I remember correctly though he wasn't able to make it work for
>> rebasing which is how I made these patches :/.
>
> What exactly was the problem?

https://newartisans.com/2009/02/building-a-better-pre-commit-hook-for-git/

https://newartisans.com/2009/02/the-saga-of-rebase-versus-merge/


I haven't actually tried implementing any of this and the blog posts
date back to 2009 so there might be easy fixes for these issues
available now.

In the first link John Wiegley describes the issue with creating the
pre-commit hook in the first place (wanting to test only the commit made
while having a dirty worktree).

In the second link he discovers his solution is inadequate for his
workflow as the hooks aren't run on rebase.