emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Possible bug with columnview and active dates
@ 2020-05-10 15:29 Org Mode
  2020-05-10 19:13 ` Kyle Meyer
  0 siblings, 1 reply; 10+ messages in thread
From: Org Mode @ 2020-05-10 15:29 UTC (permalink / raw)
  To: emacs-orgmode@gnu.org

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bug with columnview and active dates
  2020-05-10 15:29 Possible bug with columnview and active dates Org Mode
@ 2020-05-10 19:13 ` Kyle Meyer
  2020-05-10 20:11   ` Kyle Meyer
  0 siblings, 1 reply; 10+ messages in thread
From: Kyle Meyer @ 2020-05-10 19:13 UTC (permalink / raw)
  To: orgmode, emacs-orgmode


> 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.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bug with columnview and active dates
  2020-05-10 19:13 ` Kyle Meyer
@ 2020-05-10 20:11   ` Kyle Meyer
  2020-05-10 20:55     ` Kyle Meyer
  2020-05-10 21:31     ` [PATCH] manual: Fix special property descriptions for DEADLINE/SCHEDULED Kyle Meyer
  0 siblings, 2 replies; 10+ messages in thread
From: Kyle Meyer @ 2020-05-10 20:11 UTC (permalink / raw)
  To: orgmode, emacs-orgmode

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: Possible bug with columnview and active dates
  2020-05-10 20:11   ` Kyle Meyer
@ 2020-05-10 20:55     ` Kyle Meyer
  2020-05-11  6:59       ` Nicolas Goaziou
  2020-05-10 21:31     ` [PATCH] manual: Fix special property descriptions for DEADLINE/SCHEDULED Kyle Meyer
  1 sibling, 1 reply; 10+ messages in thread
From: Kyle Meyer @ 2020-05-10 20:55 UTC (permalink / raw)
  To: orgmode, emacs-orgmode

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)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] manual: Fix special property descriptions for DEADLINE/SCHEDULED
  2020-05-10 20:11   ` Kyle Meyer
  2020-05-10 20:55     ` Kyle Meyer
@ 2020-05-10 21:31     ` Kyle Meyer
  1 sibling, 0 replies; 10+ messages in thread
From: Kyle Meyer @ 2020-05-10 21:31 UTC (permalink / raw)
  To: orgmode, emacs-orgmode

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




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: Possible bug with columnview and active dates
  2020-05-10 20:55     ` Kyle Meyer
@ 2020-05-11  6:59       ` Nicolas Goaziou
  2020-05-11 13:24         ` Kyle Meyer
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Goaziou @ 2020-05-11  6:59 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: emacs-orgmode, orgmode

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bug with columnview and active dates
  2020-05-11  6:59       ` Nicolas Goaziou
@ 2020-05-11 13:24         ` Kyle Meyer
  2020-05-12  2:08           ` [PATCH] colview: Display active time stamps as inactive Kyle Meyer
  0 siblings, 1 reply; 10+ messages in thread
From: Kyle Meyer @ 2020-05-11 13:24 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: orgmode

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.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] colview: Display active time stamps as inactive
  2020-05-11 13:24         ` Kyle Meyer
@ 2020-05-12  2:08           ` Kyle Meyer
  2020-05-13  2:51             ` Kyle Meyer
  0 siblings, 1 reply; 10+ messages in thread
From: Kyle Meyer @ 2020-05-12  2:08 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: orgmode

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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] colview: Display active time stamps as inactive
  2020-05-12  2:08           ` [PATCH] colview: Display active time stamps as inactive Kyle Meyer
@ 2020-05-13  2:51             ` Kyle Meyer
  2020-05-13  8:01               ` Nicolas Goaziou
  0 siblings, 1 reply; 10+ messages in thread
From: Kyle Meyer @ 2020-05-13  2:51 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: orgmode

Kyle Meyer writes:

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

Pushed (7684b59c7, 75902a473).


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] colview: Display active time stamps as inactive
  2020-05-13  2:51             ` Kyle Meyer
@ 2020-05-13  8:01               ` Nicolas Goaziou
  0 siblings, 0 replies; 10+ messages in thread
From: Nicolas Goaziou @ 2020-05-13  8:01 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: emacs-orgmode, orgmode

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-13  8:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-10 15:29 Possible bug with columnview and active dates Org Mode
2020-05-10 19:13 ` Kyle Meyer
2020-05-10 20:11   ` Kyle Meyer
2020-05-10 20:55     ` Kyle Meyer
2020-05-11  6:59       ` Nicolas Goaziou
2020-05-11 13:24         ` Kyle Meyer
2020-05-12  2:08           ` [PATCH] colview: Display active time stamps as inactive Kyle Meyer
2020-05-13  2:51             ` Kyle Meyer
2020-05-13  8:01               ` Nicolas Goaziou
2020-05-10 21:31     ` [PATCH] manual: Fix special property descriptions for DEADLINE/SCHEDULED Kyle Meyer

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).