Re: columnview dynamic block - different time summing behaviour for EFFORT and CLOCKSUM

[Alexander Adolf <alexander.adolf@condition-alpha.com>]

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

Ihor Radchenko <yantar92@posteo.net> writes:

> [...]
>>> If you can, please add some more tests like mine checking
>>> `org-columns--clean-item'.
> [...]

I have added one test for each new feature (headline linkification, and
formatting function). Also, there's a third patch, which moves the call
to `org-quote-vert' to the cleanup function `org-columns--clean-item' as
you suggested.

Updated patches below.


One more thing caught my eye in `org-dblock-write:columnview' though.
When running the currently (i.e. before my patches) last test in
test-org-colview.el, the one with `org-columns-default-format' set to
"%ITEM %DEADLINE(d) %SCHEDULED(s) %TIMESTAMP(t)", `org-with-wide-buffer'
appears to modify the buffer and advances point by one (before the call
point is at 97, after the call at 98). In all other tests,
`org-with-wide-buffer' does not move point. In
`org-dblock-write:columnview' I have thus moved the sampling of point
for passing to the formatting function to after the call to
`org-with-wide-buffer'.


Many thanks and looking forward to your thoughts,

  --alexander



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-org-colview.el-Add-formatter-parameter-to-colvi.patch --]
[-- Type: text/x-patch, Size: 8958 bytes --]

From 2bb19d9000f29d56295b70754f89f927e7a5740c Mon Sep 17 00:00:00 2001
From: Alexander Adolf <alexander.adolf@condition-alpha.com>
Date: Sun, 14 Apr 2024 18:14:05 +0200
Subject: [PATCH 1/3] lisp/org-colview.el: Add formatter parameter to colview
 dynamic block

* lisp/org-colview.el (org-dblock-write:column view): Factor out the
existing formatting code to new function
`org-columns-dblock-write-default', and honour new dblock parameter
:formatter for specifying a different formatting function.
(org-columns-dblock-write-default): New function with current
formatting code.
(org-columns--capture-view): Amend docstring to better explain the
format of the data being passed to the formatting function.
(org-clock-clocktable-formatter): New option to define a global
default formatting function, defaulting to the current behaviour.
* testing/lisp/test-org-colview.el (test-org-colview/dblock): New test
for formatting function.
(test-org-colview/dblock-formatter): New function used in formatting
test.
* doc/org-manual.org (Capturing column view): Describe new :formatter
parameter.
* etc/ORG-NEWS (New option ~org-columns-dblock-formatter~): Announce
new option.
(=colview= dynamic block supports custom formatting function):
Describe new custom formatting function feature.
---
 doc/org-manual.org               |  7 ++++++
 etc/ORG-NEWS                     | 29 ++++++++++++++++++++++++
 lisp/org-colview.el              | 38 +++++++++++++++++++++++++++-----
 testing/lisp/test-org-colview.el | 20 +++++++++++++++++
 4 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index d24230568..000783772 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -6014,6 +6014,13 @@ This dynamic block has the following parameters:
   Specify a column attribute (see [[*Column attributes]]) for the dynamic
   block.
 
+- =:formatter= ::
+
+  #+cindex: @samp{formatter}, dynamic block parameter
+  #+vindex: org-columns-dblock-formatter
+  A function to format column view data and insert it into the buffer.
+  See the option ~org-columns-dblock-formatter~.
+
 The following commands insert or update the dynamic block:
 
 - ~org-columns-insert-dblock~ ::
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index dbf849422..84ea7cde9 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -999,6 +999,16 @@ even though it does not have its own ID.  By giving files top-level id
 properties, links to headlines in the file can also be made more
 robust by using the file id instead of the file path.
 
+*** New option ~org-columns-dblock-formatter~
+
+=colview= dynamic blocks now understand a new ~:formatter~ parameter
+to use a specific function for formatting and inserting the contents
+of the dynamic block. This new option can be used to set the global
+default formatting function that will be used for =colview= dynamic
+blocks that do not specify any ~:formatter~ parameter. Its default
+value (the new function ~org-columns-dblock-write-default~) yields the
+previous (fixed) formatting behaviour.
+
 ** New features
 *** ~org-paste-subtree~ now handles =C-u= and =C-u C-u= prefix arguments specially
 
@@ -1035,6 +1045,25 @@ Example:
 : | PROYECTO EMACS |
 : #+END:
 
+*** =colview= dynamic block supports custom formatting function
+
+The =colview= dynamic block understands a new ~:formatter~ parameter,
+which specifies a user-supplied function to format and insert the data
+in the dynamic block.
+
+A global default formatting function for =colview= dynamic blocks can
+be set via the new option ~org-columns-dblock-formatter~ which
+defaults to the new function ~org-columns-dblock-write-default~, that
+implements the previous (fixed) formatting behaviour. Hence, the
+default behaviour is identical to previous versions.
+
+The global default function can be overridden for any given =colview=
+dynamic block individually by specifying a custom formatter function
+using the new ~:formatter~ parameter on the block's =BEGIN= line.
+
+This new feature replicates the ~:formatter~ option already available
+for =clocktable= dynamic blocks.
+
 *** =ob-tangle.el=: New flag to remove tangle targets before writing
 
 When ~org-babel-tangle-remove-file-before-write~ is set to ~t~ the
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 4718d1db7..6241fa3ba 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -123,6 +123,12 @@ in `org-columns-summary-types-default', which see."
 			      (function :tag "Summarize")
 			      (function :tag "Collect")))))
 
+(defcustom org-columns-dblock-formatter #'org-columns-dblock-write-default
+  "Function to format data in column view dynamic blocks.
+For more information, see `org-columns-dblock-write-default'."
+  :group 'org-properties
+  :package-version '(Org . "9.7")
+  :type 'function)
 
 \f
 ;;; Column View
@@ -1442,9 +1448,13 @@ that will be excluded from the resulting view.  FORMAT is a
 format string for columns, or nil.  When LOCAL is non-nil, only
 capture headings in current subtree.
 
-This function returns a list containing the title row and all
-other rows.  Each row is a list of fields, as strings, or
-`hline'."
+This function returns a list containing the title row and all other
+rows.  Each row is either a list, or the symbol `hline'.  The first list
+is the heading row as a list of strings with the column titles according
+to FORMAT.  All subsequent lists each represent a body row as a list
+whose first element is an integer indicating the outline level of the
+entry, and whose remaining elements are strings with the contents for
+the columns according to FORMAT."
   (org-columns (not local) format)
   (goto-char org-columns-top-level-marker)
   (let ((columns (length org-columns-current-fmt-compiled))
@@ -1545,7 +1555,13 @@ PARAMS is a property list of parameters:
 `:vlines'
 
     When non-nil, make each column a column group to enforce
-    vertical lines."
+    vertical lines.
+
+`:formatter'
+
+    A function to format the data and insert it into the
+    buffer.  Overrides the default formatting function set in
+    `org-columns-dblock-formatter'."
   (let ((table
 	 (let ((id (plist-get params :id))
 	       view-file view-pos)
@@ -1573,7 +1589,17 @@ PARAMS is a property list of parameters:
 					 (plist-get params :exclude-tags)
 					 (plist-get params :format)
 					 view-pos)))))
-        (width-specs
+        (formatter (or (plist-get params :formatter)
+                       org-columns-dblock-formatter
+                       #'org-columns-dblock-write-default)))
+    (funcall formatter (point) table params)))
+
+(defun org-columns-dblock-write-default (ipos table params)
+  "Write out a columnview table at position IPOS in the current buffer.
+TABLE is a table with data as produced by `org-columns--capture-view'.
+PARAMS is the parameter property list obtained from the dynamic block
+definition."
+  (let ((width-specs
          (mapcar (lambda (spec) (nth 2 spec))
                  org-columns-current-fmt-compiled)))
     (when table
@@ -1616,6 +1642,8 @@ PARAMS is a property list of parameters:
         ;; to the resulting table, adding alignment field as the first
         ;; row.
         (push (mapcar (lambda (width) (when width (format "<%d>" width))) width-specs) table))
+      ;; now insert the table into the buffer
+      (goto-char ipos)
       (let ((content-lines (org-split-string (plist-get params :content) "\n"))
 	    recalc)
 	;; Insert affiliated keywords before the table.
diff --git a/testing/lisp/test-org-colview.el b/testing/lisp/test-org-colview.el
index 7f0aa763e..bcf4e897f 100644
--- a/testing/lisp/test-org-colview.el
+++ b/testing/lisp/test-org-colview.el
@@ -1398,6 +1398,13 @@
 \f
 ;;; Dynamic block
 
+(defun test-org-colview/dblock-formatter (ipos table params)
+  "User-defined columnview dblock formatting function."
+  (goto-char ipos)
+  (insert-before-markers "Hello columnview!" "\n")
+  (insert-before-markers (format "table has %d rows" (length table)) "\n")
+  (insert-before-markers (format "there are %d parameters" (/ (length params) 2))))
+
 (ert-deftest test-org-colview/dblock ()
   "Test the column view table."
   (should
@@ -1703,6 +1710,19 @@ SCHEDULED: <2020-05-11 Mon> DEADLINE: <2020-05-14 Thu>
      (let ((org-columns-default-format
 	    "%ITEM %DEADLINE(d) %SCHEDULED(s) %TIMESTAMP(t)"))
        (org-update-dblock))
+     (buffer-substring-no-properties (point) (point-max)))))
+  ;; custom formatting function
+  (should
+   (equal
+    "#+BEGIN: columnview :formatter test-org-colview/dblock-formatter
+Hello columnview!
+table has 3 rows
+there are 4 parameters
+#+END:"
+    (org-test-with-temp-text
+     "* H\n<point>#+BEGIN: columnview :formatter test-org-colview/dblock-formatter\n#+END:"
+     (let ((org-columns-default-format "%ITEM"))
+       (org-update-dblock))
      (buffer-substring-no-properties (point) (point-max))))))
 
 (provide 'test-org-colview)
-- 
2.39.3 (Apple Git-146)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-lisp-org-colview.el-Add-link-parameter-to-colview-dy.patch --]
[-- Type: text/x-patch, Size: 4827 bytes --]

From eb7c61bea2c644fba79aff2d71e93a1a3004f0b9 Mon Sep 17 00:00:00 2001
From: Alexander Adolf <alexander.adolf@condition-alpha.com>
Date: Mon, 15 Apr 2024 18:01:40 +0200
Subject: [PATCH 2/3] lisp/org-colview.el: Add link parameter to colview
 dynamic block

* lisp/org-colview.el (org-columns--capture-view): Add new link
parameter, which when non-nil causes ITEM headlines to be linked to
their origins.
(org-dblock-write:columnview): Pass new link parameter to
`org-columns--capture-view', and explain its use in the docstring.
* testing/lisp/test-org-colview.el (test-org-colview/dblock): add
new test for link feature
* doc/org-manual.org (Capturing column view): Describe new :link
parameter.
* etc/ORG-NEWS (=colview= dynamic block can link to headlines):
Describe new link feature.
---
 doc/org-manual.org               |  5 +++++
 etc/ORG-NEWS                     |  6 ++++++
 lisp/org-colview.el              | 20 ++++++++++++++++----
 testing/lisp/test-org-colview.el | 12 ++++++++++++
 4 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 000783772..a77492971 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -6009,6 +6009,11 @@ This dynamic block has the following parameters:
 
   When non-~nil~, indent each =ITEM= field according to its level.
 
+- =:link= ::
+
+  When non-~nil~, link the =ITEM= headlines in the table to their
+  origins.
+
 - =:format= ::
 
   Specify a column attribute (see [[*Column attributes]]) for the dynamic
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 84ea7cde9..b4aea10b8 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -1064,6 +1064,12 @@ using the new ~:formatter~ parameter on the block's =BEGIN= line.
 This new feature replicates the ~:formatter~ option already available
 for =clocktable= dynamic blocks.
 
+*** =colview= dynamic block can link to headlines
+
+The =colview= dynamic block understands a new ~:link~ parameter, which
+when non-~nil~ causes =ITEM= headlines in the table to be linked to
+their origins.
+
 *** =ob-tangle.el=: New flag to remove tangle targets before writing
 
 When ~org-babel-tangle-remove-file-before-write~ is set to ~t~ the
diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 6241fa3ba..64ed8c16a 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1557,6 +1557,10 @@ PARAMS is a property list of parameters:
     When non-nil, make each column a column group to enforce
     vertical lines.
 
+`:link'
+
+    Link the item headlines in the table to their origins.
+
 `:formatter'
 
     A function to format the data and insert it into the
@@ -1599,9 +1603,10 @@ PARAMS is a property list of parameters:
 TABLE is a table with data as produced by `org-columns--capture-view'.
 PARAMS is the parameter property list obtained from the dynamic block
 definition."
-  (let ((width-specs
-         (mapcar (lambda (spec) (nth 2 spec))
-                 org-columns-current-fmt-compiled)))
+  (let ((link (plist-get params :link))
+	(width-specs
+	 (mapcar (lambda (spec) (nth 2 spec))
+		 org-columns-current-fmt-compiled)))
     (when table
       ;; Prune level information from the table.  Also normalize
       ;; headings: remove stars, add indentation entities, if
@@ -1625,7 +1630,14 @@ definition."
 			   (and (numberp hlines) (<= level hlines))))
 	      (push 'hline new-table))
 	    (when item-index
-	      (let ((item (org-columns--clean-item (nth item-index (cdr row)))))
+	      (let* ((raw (nth item-index (cdr row)))
+		     (cleaned (org-columns--clean-item raw))
+		     (item (if (not link) cleaned
+			     (let ((search (org-link-heading-search-string raw)))
+			       (org-link-make-string
+				(if (not (buffer-file-name)) search
+				  (format "file:%s::%s" (buffer-file-name) search))
+				cleaned)))))
 		(setf (nth item-index (cdr row))
 		      (if (and indent (> level 1))
 			  (concat "\\_" (make-string (* 2 (1- level)) ?\s) item)
diff --git a/testing/lisp/test-org-colview.el b/testing/lisp/test-org-colview.el
index bcf4e897f..6b603c31b 100644
--- a/testing/lisp/test-org-colview.el
+++ b/testing/lisp/test-org-colview.el
@@ -1723,6 +1723,18 @@ there are 4 parameters
      "* H\n<point>#+BEGIN: columnview :formatter test-org-colview/dblock-formatter\n#+END:"
      (let ((org-columns-default-format "%ITEM"))
        (org-update-dblock))
+     (buffer-substring-no-properties (point) (point-max)))))
+  ;; test headline linkification
+  (should
+   (equal
+    "#+BEGIN: columnview :link t
+| ITEM |
+|------|
+| [[*H][H]]    |
+#+END:"
+    (org-test-with-temp-text
+     "* H\n<point>#+BEGIN: columnview :link t\n#+END:"
+     (let ((org-columns-default-format "%ITEM")) (org-update-dblock))
      (buffer-substring-no-properties (point) (point-max))))))
 
 (provide 'test-org-colview)
-- 
2.39.3 (Apple Git-146)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-lisp-org-colview.el-Move-cleanup-code-to-cleanup-fun.patch --]
[-- Type: text/x-patch, Size: 1895 bytes --]

From 03334c493fec07d0571dc9298ce11d96fd8ba185 Mon Sep 17 00:00:00 2001
From: Alexander Adolf <alexander.adolf@condition-alpha.com>
Date: Tue, 23 Apr 2024 17:43:08 +0200
Subject: [PATCH 3/3] lisp/org-colview.el: Move cleanup code to cleanup
 function

* lisp/org-colview.el (org-columns--clean-item): Additionally process
the result with `org-quote-vert'.
(org-columns--capture-view): Remove call to `org-quote-vert'.

`org-columns--clean-item' is used when formatting and inserting column
view dynamic blocks. By moving the call to `org-quote-vert' to this
function, it can be used by all formatting functions, including
user-supplied ones, to format content to be safe for inclusion in a
table.
---
 lisp/org-colview.el | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 64ed8c16a..e934ae67a 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1467,11 +1467,10 @@ the columns according to FORMAT."
 	   (dotimes (i columns)
 	     (let* ((col (+ (line-beginning-position) i))
 		    (p (get-char-property col 'org-columns-key)))
-	       (push (org-quote-vert
-		      (get-char-property col
-					 (if (string= p "ITEM")
-					     'org-columns-value
-					   'org-columns-value-modified)))
+	       (push (get-char-property col
+					(if (string= p "ITEM")
+					    'org-columns-value
+					  'org-columns-value-modified))
 		     row)))
 	   (unless (or
 		    (and skip-empty
@@ -1503,7 +1502,9 @@ an inline src-block."
 	'(footnote-reference inline-babel-call inline-src-block target
 			     radio-target statistics-cookie)
       #'org-element-extract)
-    (org-no-properties (org-element-interpret-data data))))
+    (org-quote-vert
+     (org-no-properties
+      (org-element-interpret-data data)))))
 
 ;;;###autoload
 (defun org-dblock-write:columnview (params)
-- 
2.39.3 (Apple Git-146)