bug#55900: [PATCH] Hierarchy – Defer the Computation of Children

[Eli Zaretskii]

> Date: Sun, 12 Jun 2022 09:07:37 +0000 (UTC)
> From: Jaft <jaft.r@outlook.com>
> Cc: "55900@debbugs.gnu.org" <55900@debbugs.gnu.org>
> 
> Hmm; I tried to attach the patch with Gnus but it doesn't look like it 
> actually added it…. Sending again, via the browser this time.

Thanks.  Please see the few minor comments below.

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1756,6 +1756,15 @@ Enabling this minor mode turns on hiding header 
> material, like
>  If non-nil, files untracked by a VCS are considered to be part of
>  the project by a VC project based on that VCS.
>  
> +** Hierarchy
> +
> ++++
> +*** Tree Display can delay computation of children
> +'hierarchy-add-tree' and 'hierarchy-add-trees' have an optional
> +argument, 'delay-children-p', which allows storing 'childrenfn' to the
> +nodes of a tree-widget display to be activated and computed only when
> +the users expands the node.

The first line (the heading) should end in a dot '.'

The description itself is OK, but it sounds too detailed for NEWS.
I'd suggest to make it shorter by dropping the reference to the
optional argument's name and the "storing" part.  It should be enough
to say "which allows tree-widget display to be activated and computed
only when the user expands the node".

>  (declare-function widget-convert "wid-edit")
> +(defun hierarchy--create-delayed-tree-widget (elem labelfn indent childrenfn)
> +  "Return a list of tree-widgets for the children generated by calling
> +CHILDRENFN on ELEM.

The first line of a doc string should be a complete sentence.  This is
an internal function, so you could make the first sentence be less
detailed.

> --- a/test/lisp/emacs-lisp/hierarchy-tests.el
> +++ b/test/lisp/emacs-lisp/hierarchy-tests.el
> @@ -552,5 +552,146 @@
>      (hierarchy-sort organisms)
>      (should (equal (hierarchy-roots organisms) '(animal plant)))))
>  
> +(defun hierarchy-examples-delayed--find-number (num)
> +  "Find a number, NUM, by adding 1s together until you reach it.
> +This is entire contrived and mostly meant to be purposefully inefficient to
> +not be possible on a large scale.
> +Running the number 200 causes this function to crash; running this function 
> in
> +`hierarchy-add-tree' with a root of 80 and no delayed children causes that to
> + crash."

Can you explain better what is the purpose of this function in the
context of the tests which use it?