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

[mousebot]

Hi Damien,

I tried our your suggestion in my library, and it works fine.

My initial query was also a way of asking if there is any (other) way in 
hierarchy.el of printing while respecting indentation. I don't know the library 
well at all. I guess the answer is no, given your response?

I'm unsure as to how to refactor hierarchy-print, as I don't understand what 
you think it ought to do differently once hierarchy-print-line is added.

I also wonder if it's perhaps a little confusing to have an arg 'to-string' 
that is now likely to be a call to `hierarchy-labelfn-indent`, whereas in 
`hierarchy-print` `to-string` is a function handed to hierarchy-labelfn-indent 
as an argument.

In that case, perhaps -print-line should have the same call to 
`hierarchy-labelfn-indent` in its body, so that the to-string arg is of a 
similar form to -print?

Finally, is there anything wrong with simply making the indent arg in the lambda in 
my original suggestion '&optional'?

i.e.

(defun hierarchy-print (hierarchy &optional to-string indent-string)
  (let ((to-string (or to-string (lambda (item) (format "%s" item)))))
    (hierarchy-map
     (hierarchy-labelfn-indent (lambda (item &optional indent)
                                 (insert (funcall to-string item indent) "\n"))
                               indent-string)
     hierarchy)))

Maybe sending 'nil' as an argument to funcall is bad news?

I don't really understand why the -print function shouldn't take an 
indent-string argument, but it's your pkg not mine.

kind regards,
m


On 7/26/23 18:16, Damien Cassou wrote:
> Hi,
>
> you are right that something isn't good enough. The fix you suggest
> fixes the problem. Unfortunately, it is backward incompatible: all
> usages of `hierarchy-print' passing a custom value for TO-STRING will
> now fail because TO-STRING should now read a second argument.
> Additionally, I think it shouldn't be the responsibility of
> `hierarchy-print' to take care of the indentation.
>
> Had I been smart enough, I would probably have written `hierarchy-print'
> this way instead:
>
>    (defun hierarchy-print (hierarchy &optional to-string)
>      (let ((to-string (or to-string (lambda (item) (format "%s" item)))))
>        (hierarchy-map
>         (lambda (item indent)
>           (insert (funcall to-string item ident) "\n"))
>         hierarchy)))
>
> That is:
> - TO-STRING takes 2 parameters
>
> - indentation should be decided by the user through a dedicated value
>    TO-STRING value, e.g., with
>    (hierarchy-labelfn-indent (lambda (item) (format "%s" item)))
>
> But this implementation also breaks backward compatibility. What I
> suggest:
>
> 1. Implement `hierarchy-print-line' with the implementation above (this
>     way we have a good implementation anyone can use)
>
> 2. Refactor and simplify `hierarchy-print` to avoid duplication with
>     `hierarchy-print-line' but keeping backward compatibility.
>
> What do you think? Would you mind sending a patch?
>
> (please keep me on CC)
>
> Best