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

[Ihor Radchenko]

Alexander Adolf <alexander.adolf@condition-alpha.com> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> [...]
>> Calling `org-columns--clean-item' is a must to create a valid table.
>
> True.
>
> Additionally, it would seem advisable to call `org-quote-vert' on the
> data, too, as `org-columns--clean-item' does not take care of vertical
> bars? This is done in a previous step in `org-columns--capture-view',
> however, so that the vertical bars get converted to "\vert" before the
> formatting function gets called.
>
> `org-link-heading-search-string', and `org-link-make-string' (both
> called from the formatting function _after_ `org-columns--clean-item')
> OTOH take care of the link's path and description parts being
> appropriate for a link.

It would make sense then to include `org-quote-vert' call into
`org-columns--clean-item' then.

> Kindly find updated patches below. I hope to have succeeded in
> addressing all your comments; that was my intention at least.

Thanks!

>  (defun org-columns--capture-view (maxlevel match skip-empty exclude-tags 
> format local)
>    "Get the column view of the current buffer.
>...
> +When LOCAL is non-nil, only capture headings in current subtree.  When
> +LINK is non-nil, item headlines will be linked to their origins.

Looks like you removed the LINK parameter, but forgot to remove its
description from the docstring.
  
> +                            (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))

This will unconditionally generate file: links, even when the dynamic
block only refers to headings in the same buffer. The clock tables do
use internal links when appropriate (see `org-clock-get-table-data').

I am attaching a patch containing test case making sure that internal
links are generated when appropriate. The test case is failing with
the latest version of your patch.

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

>From 3961072d80883aef5da21d8d6ba10213778ff32f Mon Sep 17 00:00:00 2001
Message-ID: 
<3961072d80883aef5da21d8d6ba10213778ff32f.1713871396.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Tue, 23 Apr 2024 14:22:44 +0300
Subject: [PATCH] org-colview: Add test for the new :link parameter

* testing/lisp/test-org-colview.el (test-org-colview/dblock): New
test case.
---
 testing/lisp/test-org-colview.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/testing/lisp/test-org-colview.el b/testing/lisp/test-org-colview.el
index 7f0aa763e..872a61753 100644
--- a/testing/lisp/test-org-colview.el
+++ b/testing/lisp/test-org-colview.el
@@ -1422,6 +1422,18 @@ (ert-deftest test-org-colview/dblock ()
         "* H\n:PROPERTIES:\n:A: 1\n:END:\n<point>#+BEGIN: columnview\n#+END:"
       (let ((org-columns-default-format "%ITEM %A")) (org-update-dblock))
       (buffer-substring-no-properties (point) (point-max)))))
+  ;; Test `:link' parameter.
+  (should
+   (equal
+    "#+BEGIN: columnview
+| ITEM |
+|------|
+| [[*H][H]]    |
+#+END:"
+    (org-test-with-temp-text
+        "* H\n<point>#+BEGIN: columnview\n#+END:"
+      (let ((org-columns-default-format "%ITEM")) (org-update-dblock))
+      (buffer-substring-no-properties (point) (point-max)))))
   ;; Test column widths.
   (should
    (equal
-- 
2.44.0

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>