Morgan Smith <[email protected]> writes:

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

Maybe worth marking this with FIXME:
Also, you can try fixing this bug.
See 32.5 Marker Insertion Types and `org-agenda-new-marker'.

> +  (unless (org-element-type-p headline '(headline inlinetask))
> +    (error "Argument must be a headline"))

... or inlinetask.

> +  (and
> +   (org-element-contents-begin headline) ;; empty headline

Maybe non-empty?

> +            ;; XXX: using these arguments would be more intuitive

FIXME:

> Subject: [PATCH 11/12] Rewrite `org-clock-get-table-data' using org-element
> ...
> +     (org-element-cache-map

This will likely slow things down.
The original code uses `next-single-property-change' which will
automatically skip headings that do not contain clocks (as determine by
`org-clock-sum'). The new code will again go through every single
headline and inlinetask.

> +      (lambda (elm)
> +        (when-let*
> +            ((time
> +              (let ((time (get-text-property (org-element-begin elm)
> +                                             :org-clock-minutes)))
> +                (when (and time (> time 0))
> +                  time)))

Can simply do
(time (get-text-property ...))
(time (when (> time 0) time))

> +          (list level
> +                title
> +                (and tags (org-element-property :tags elm))

This is not quite the same as `org-get-tags' (without LOCAL argument).

> +                (and timestamp
> +                     (cl-some
> +                      (lambda (p) (org-entry-get (org-element-begin elm) p))

You can pass ELM directly to `org-entry-get'.

> Subject: [PATCH 12/12] Clocktables: Indent inline tasks under their heading
>
> * lisp/org-clock.el (lisp/org-clock.el): Inrease the level of inline
> tasks by 1.
> * testing/lisp/test-org-clock.el
> (test-org-clock/clocktable/inlinetask/insert)
> (test-org-clock/clocktable/inlinetask/open-clock): Adjust tests to new
> behavior.

Please explain the rationale for the new behavior in the commit
message. Also, it is a user-facing change (table presentation) and
should be announced in ORG-NEWS.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Reply via email to