Re: hierarchy-print doesn't allow sending indent-string arg to hierarchy-map

[Damien Cassou]

mousebot <mousebot@riseup.net> writes:
> perhaps there is some other code it run it against also?


I don't understand this sentence.


> a patch is attached, formatted with magit from a magit diff. is that
> applicable, or is a different format required?


Patches should usually be based on the root of the project so I would
expect to see "a/lisp/emacs-lisp/hierarchy.el" instead of
"a/hierarchy.el" in the patch. Also, the patch shouldn't be a plain diff
but should also contain information that the commit needs (e.g., author,
date, message).

To create a patch, my approach is to first create a dedicated branch,
then a commit (with a proper message) and finally to ask Magit to
generate a patch for the commit (with "M-x magit-patch-create" or "W c
c" and the cursor on the commit in the Magit status buffer).

Please read "** Commit messages" in the CONTRIBUTE file at the root of
the emacs tree to know how to write your commit message.

> --- a/hierarchy.el
> +++ b/hierarchy.el
> @@ -475,10 +475,31 @@ indentation value (a number) as input."
>  
>  Use TO-STRING to convert each element to a string.  TO-STRING is
>  a function taking an item of HIERARCHY as input and returning a
> -string.  If nil, TO-STRING defaults to a call to `format' with \"%s\"."
> +string.


I think the last sentence should be kept, it still makes sense.


> +
> +Calls `hierarchy-print-line' with `hierarchy-labelfn-indent' as
> +second argument."


This is an implementation detail we don't to mention here.

What is not an implementation detail is that the hierarchy's elements
are indented with a 2-space string. We could add a sentence about that.

We could also reference the new function `hierarchy-print-line' to tell
the user that this later one gives more configuration freedom but is a
little more complicated to use.


>    (let ((to-string (or to-string (lambda (item) (format "%s"
>    item)))))
> +    (hierarchy-print-line
> +     hierarchy
> +     (hierarchy-labelfn-indent
> +      (lambda (item _)
> +        (funcall to-string item))))))
> +
> +(defun hierarchy-print-line (hierarchy &optional labelfn)
> +  "Insert HIERARCHY in current buffer as plain text.
> +
> +Use LABELFN to convert each element to a string.  LABELFN is
> +a function taking an item of HIERARCHY as input


"as well as an indentation level as number".


> and returning a
> +string.  If nil, LABELFN defaults to a call to `format' with \"%s\".


I don't think LABELFN should have a default value in this function. It
is lower-level than hierarchy-print. Also, the user will have to
customize it in many cases to get indentation.


> +
> +This function is not responsible for indentation, but it can be
> +achieved by providing a function such as
> +`hierarchy-labelfun-indent' for LABELFN."
> +  (let ((labelfn (or labelfn (lambda (item) (format "%s" item)))))
>      (hierarchy-map
> -     (hierarchy-labelfn-indent (lambda (item _) (insert (funcall
> to-string item) "\n")))


indentation shouldn't be the responsibility of this function. This
function is lower-level.

Could you please also add a unit test or two for the new function?

Thank you for your work

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill