Possible bug with columnview and active dates

Possible bug with columnview and active dates

[Org Mode]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

Hello,

I posted this question here: https://emacs.stackexchange.com/questions/58337/how-remove-active-date-from-a-dynamic-block-using-columnview regarding removing the active dates from a columnview. A user replied suggesting this could maybe be a bug.

According to the docs: https://orgmode.org/manual/Special-Properties.html#Special-Properties Using DEADLINE or SCHEDULED should not include the angular brackets.

Thank you!

[-- Attachment #2: Type: text/html, Size: 1006 bytes --]

Re: Possible bug with columnview and active dates

[Kyle Meyer]

> I posted this question here:
> https://emacs.stackexchange.com/questions/58337/how-remove-active-date-from-a-dynamic-block-using-columnview
> regarding removing the active dates from a columnview. A user replied
> suggesting this could maybe be a bug.
>
> According to the docs:
> https://orgmode.org/manual/Special-Properties.html#Special-Properties
> Using DEADLINE or SCHEDULED should not include the angular brackets.

Thanks for the report.  There is certainly a document/code mismatch.
Digging a bit, it seems that brackets started being included quite a
while ago: 8d8ad9838 (Rewrite `org-entry-properties', 2014-09-30).  I
didn't spot anything in that commit that suggests that it was an
intended change.

Re: Possible bug with columnview and active dates

[Kyle Meyer]

Kyle Meyer writes:

>> According to the docs:
>> https://orgmode.org/manual/Special-Properties.html#Special-Properties
>> Using DEADLINE or SCHEDULED should not include the angular brackets.
>
> Thanks for the report.  There is certainly a document/code mismatch.
> Digging a bit, it seems that brackets started being included quite a
> while ago: 8d8ad9838 (Rewrite `org-entry-properties', 2014-09-30).  I
> didn't spot anything in that commit that suggests that it was an
> intended change.

While I think it's be fair to consider this a regression, I'm leery of
restoring the original behavior of stripping the brackets given how long
the current behavior has been around.  Instead I think it'd be better to
update the documentation and tweak the output on colview's end.

However, for the record, when I restored this behavior locally, here
were the adjustments needed to make the tests pass.  (Of course,
restoring the behavior likely introduces other issues that are not
caught be the tests, not to mention third-party code that relies on
brackets being included.)

diff --git a/lisp/org.el b/lisp/org.el
index 4d1a6a6b4..18a358c85 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -10817,7 +10817,7 @@ (defun org-auto-repeat-maybe (done-word)
       ;; a SCHEDULED time-stamp without one is removed, as they are no
       ;; longer relevant.
       (save-excursion
-	(let ((scheduled (org-entry-get (point) "SCHEDULED")))
+	(let ((scheduled (format "<%s>" (org-entry-get (point) "SCHEDULED"))))
 	  (when (and scheduled (not (string-match-p org-repeat-re scheduled)))
 	    (org-remove-timestamp-with-keyword org-scheduled-string))))
       ;; Update every time-stamp with a repeater in the entry.
@@ -13145,7 +13145,8 @@ (defun org-entry-properties (&optional pom which)
 			(skip-chars-forward " \t")
 			(and (looking-at org-ts-regexp-both)
 			     (push (cons (car pair)
-					 (match-string-no-properties 0))
+					 (match-string-no-properties
+					  (if (equal (car pair) "CLOSED") 0 1)))
 				   props)))))))
 	      (when specific (throw 'exit props)))
 	    (when (or (not specific)
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 29ac0a8f9..fe07ea7e3 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -5201,12 +5201,12 @@ (ert-deftest test-org/entry-properties ()
      (cdr (assoc "CLOSED" (org-entry-properties nil "CLOSED")))))
   (should
    (equal
-    "<2014-03-04 tue.>"
+    "2014-03-04 tue."
     (org-test-with-temp-text "* H\nDEADLINE: <2014-03-04 tue.>"
       (cdr (assoc "DEADLINE" (org-entry-properties nil "DEADLINE"))))))
   (should
    (equal
-    "<2014-03-04 tue.>"
+    "2014-03-04 tue."
     (org-test-with-temp-text "* H\nDEADLINE: <2014-03-04 tue.>"
       (cdr (assoc "DEADLINE" (org-entry-properties))))))
   (should-not
@@ -5214,12 +5214,12 @@ (ert-deftest test-org/entry-properties ()
      (cdr (assoc "DEADLINE" (org-entry-properties nil "DEADLINE")))))
   (should
    (equal
-    "<2014-03-04 tue.>"
+    "2014-03-04 tue."
     (org-test-with-temp-text "* H\nSCHEDULED: <2014-03-04 tue.>"
       (cdr (assoc "SCHEDULED" (org-entry-properties nil "SCHEDULED"))))))
   (should
    (equal
-    "<2014-03-04 tue.>"
+    "2014-03-04 tue."
     (org-test-with-temp-text "* H\nSCHEDULED: <2014-03-04 tue.>"
       (cdr (assoc "SCHEDULED" (org-entry-properties))))))
   (should-not

Re: Possible bug with columnview and active dates

[Kyle Meyer]

Kyle Meyer writes:

> While I think it's be fair to consider this a regression, I'm leery of

s/it's be/it's/

> restoring the original behavior of stripping the brackets given how long
> the current behavior has been around.  Instead I think it'd be better to
> update the documentation and tweak the output on colview's end.

For the "tweaking the output part", it looks this is already possible on
the user's end via org-columns-modify-value-for-display-function.
Here's a lightly tested function that I think does what you want:

    (defun my/org-columns-remove-brackets (_title value)
      (and (string-match org-ts-regexp value)
           (match-string 1 value)))
    
    (setq org-columns-modify-value-for-display-function
          #'my/org-columns-remove-brackets)

[PATCH] manual: Fix special property descriptions for DEADLINE/SCHEDULED

[Kyle Meyer]

Kyle Meyer writes:

> Instead I think it'd be better to update the documentation and tweak
> the output on colview's end.

Here's an update for the manual.  I'll apply it in a day or two if no
objections come in.

-- >8 --
Subject: [PATCH] manual: Fix special property descriptions for
 DEADLINE/SCHEDULED

* doc/org-manual.org (Special Properties): Remove claim that angular
brackets are stripped from DEADLINE and SCHEDULED time stamps, which
has not been true since v8.3, specifically 8d8ad9838.

It doesn't seem like 8d8ad9838 (Rewrite `org-entry-properties',
2014-09-30) intended to change this behavior, but let's stick with it
because some spots in Org's code base depend on it and it's likely
that third-party code does too.

Also, replace "time string" with "timestamp" in DEADLINE's description
for consistency with the other descriptions.
---
 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 92a526ba6..c20a93000 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -5169,11 +5169,11 @@ ** Special Properties
 |                | ~org-clock-sum-today~ must be run first to compute the         |
 |                | values in the current buffer.                                  |
 | =CLOSED=       | When was this entry closed?                                    |
-| =DEADLINE=     | The deadline time string, without the angular brackets.        |
+| =DEADLINE=     | The deadline timestamp.                                        |
 | =FILE=         | The filename the entry is located in.                          |
 | =ITEM=         | The headline of the entry.                                     |
 | =PRIORITY=     | The priority of the entry, a string with a single letter.      |
-| =SCHEDULED=    | The scheduling timestamp, without the angular brackets.        |
+| =SCHEDULED=    | The scheduling timestamp.                                      |
 | =TAGS=         | The tags defined directly in the headline.                     |
 | =TIMESTAMP=    | The first keyword-less timestamp in the entry.                 |
 | =TIMESTAMP_IA= | The first inactive timestamp in the entry.                     |

base-commit: 3bbaca166052c4761a1bc8227496b23dc157bf96
-- 
2.26.1

Re: Possible bug with columnview and active dates

[Nicolas Goaziou]

Hello,

Kyle Meyer <kyle@kyleam.com> writes:

>> restoring the original behavior of stripping the brackets given how long
>> the current behavior has been around.  Instead I think it'd be better to
>> update the documentation and tweak the output on colview's end.
>
> For the "tweaking the output part", it looks this is already possible on
> the user's end via org-columns-modify-value-for-display-function.
> Here's a lightly tested function that I think does what you want:
>
>     (defun my/org-columns-remove-brackets (_title value)
>       (and (string-match org-ts-regexp value)
>            (match-string 1 value)))
>     
>     (setq org-columns-modify-value-for-display-function
>           #'my/org-columns-remove-brackets)

For the record, the change was not intentional.

I agree it is not a good idea to leave a duplicate active time stamp in
a table.

IIRC, date computations in the spreadsheet return inactive timestamps.
So, if we don't want to conform to the documentation, we could insert an
inactive time-stamp instead, to be on the safe side.

WDYT?

Regards,

-- 
Nicolas Goaziou

Re: Possible bug with columnview and active dates

[Kyle Meyer]

Nicolas Goaziou writes:

> IIRC, date computations in the spreadsheet return inactive timestamps.
> So, if we don't want to conform to the documentation, we could insert an
> inactive time-stamp instead, to be on the safe side.
>
> WDYT?

Sounds good to me.  I should be able to find time in the next day or two
to look into it.

[PATCH] colview: Display active time stamps as inactive

[Kyle Meyer]

Kyle Meyer writes:

> Nicolas Goaziou writes:
>
>> IIRC, date computations in the spreadsheet return inactive timestamps.
>> So, if we don't want to conform to the documentation, we could insert an
>> inactive time-stamp instead, to be on the safe side.
>>
>> WDYT?
>
> Sounds good to me.  I should be able to find time in the next day or two
> to look into it.

Okay, here's a patch for that to go along with the documentation patch.

-- >8 --
Subject: [PATCH] colview: Display active time stamps as inactive

* lisp/org-colview.el (org-columns--displayed-value): Convert active
time stamp values to inactive time stamps to avoid duplicate entries
in the agenda.
* testing/lisp/test-org-colview.el (test-org-colview/dblock): Add
tests.
---
 lisp/org-colview.el              |  2 ++
 testing/lisp/test-org-colview.el | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 3569b231b..3e5920ccd 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -257,6 +257,8 @@ (defun org-columns--displayed-value (spec value &optional no-star)
 					  (if org-hide-leading-stars ?\s ?*))
 			     "* "))))
 	   (concat stars (org-link-display-format value))))
+	(`(,(or "DEADLINE" "SCHEDULED" "TIMESTAMP") . ,_)
+	 (replace-regexp-in-string org-ts-regexp "[\\1]" value))
 	(`(,_ ,_ ,_ ,_ nil) value)
 	;; If PRINTF is set, assume we are displaying a number and
 	;; obey to the format string.
diff --git a/testing/lisp/test-org-colview.el b/testing/lisp/test-org-colview.el
index ed75090df..7612074b5 100644
--- a/testing/lisp/test-org-colview.el
+++ b/testing/lisp/test-org-colview.el
@@ -1519,7 +1519,24 @@ (ert-deftest test-org-colview/dblock ()
     (org-test-with-temp-text
         "* H src_emacs-lisp{(+ 1 1)} 1\n<point>#+BEGIN: columnview\n#+END:"
       (let ((org-columns-default-format "%ITEM")) (org-update-dblock))
-      (buffer-substring-no-properties (point) (point-max))))))
+      (buffer-substring-no-properties (point) (point-max)))))
+  ;; Active time stamps are displayed as inactive.
+  (should
+   (equal
+    "#+BEGIN: columnview
+| ITEM | d                | s                | t                |
+|------+------------------+------------------+------------------|
+| H    | [2020-05-14 Thu] | [2020-05-11 Mon] | [2020-06-10 Wed] |
+#+END:"
+    (org-test-with-temp-text
+     "* H
+SCHEDULED: <2020-05-11 Mon> DEADLINE: <2020-05-14 Thu>
+<2020-06-10 Wed>
+<point>#+BEGIN: columnview\n#+END:"
+     (let ((org-columns-default-format
+	    "%ITEM %DEADLINE(d) %SCHEDULED(s) %TIMESTAMP(t)"))
+       (org-update-dblock))
+     (buffer-substring-no-properties (point) (point-max))))))
 
 (provide 'test-org-colview)
 ;;; test-org-colview.el ends here

base-commit: 3bbaca166052c4761a1bc8227496b23dc157bf96
-- 
2.26.2

Re: [PATCH] colview: Display active time stamps as inactive

[Kyle Meyer]

Kyle Meyer writes:

> Okay, here's a patch for that to go along with the documentation patch.

Pushed (7684b59c7, 75902a473).

Re: [PATCH] colview: Display active time stamps as inactive

[Nicolas Goaziou]

Hello,

Kyle Meyer <kyle@kyleam.com> writes:

> Kyle Meyer writes:
>
>> Okay, here's a patch for that to go along with the documentation patch.
>
> Pushed (7684b59c7, 75902a473).

Thank you!

Regards,

-- 
Nicolas Goaziou