Re: Upstreaming org-element-ast (was: Improving Emacs' iCalendar support)

[Richard Lawrence]

Ihor Radchenko <yantar92@posteo.net> writes:

> Richard Lawrence <rwl@recursewithless.net> writes:
>
>> In any case, the issue with passing nil as the &rest argument is what
>> causes most of the tests to fail. This remains so even once I change the
>> call to use (apply #'org-element-create ...) as you suggested. This *is*
>> documented in the Elisp manual, and I guess it makes sense, but I still
>> find it somewhat unintuitive to reason about.  
>
> May you please give an example illustrating the issue? I am not sure if
> I understand it fully.

OK, revisiting this after a few days...I'm not 100% sure I understand
it either.

Here is what I see: if I define ical:make-ast-node like

(defun ical:make-ast-node (type &optional props &rest children)
  ;; ...
  (apply #'org-element-create type full-props children))

which is what I was doing in the hope that I could eventually just use
defalias, most of my parser tests fail with "X may not contain `nil'"
errors as I showed in a previous message. If instead I change the
definition to:

(defun ical:make-ast-node (type props &optional children)
  ;; ...
  (apply #'org-element-create type full-props children))

which better reflects the usage pattern I need, then all the tests pass again.

The reason is that, with the former definition, children will be bound to
`(nil)' in the call to org-element-create, while in the latter it will
be bound to `nil'. The root of the problem is that

(org-element-create 'foo (list ...))
(org-element-create 'foo (list ...) nil)

are not equivalent; in particular

(org-element-contents (org-element-create 'foo (list ...))) ; => nil
(org-element-contents (org-element-create 'foo (list ...) nil)) ; => (nil)

This is the documented behavior of passing nil for a &rest argument (see
info node `(elisp)Argument list': "Note that exactly five arguments with
an explicit ‘nil’ argument provided for ‘e’ will cause that ‘nil’
argument to be passed as a list with one element, ‘(nil)’, as with any
other single value for ‘e’"). So it's not a problem per se.

But this interface to org-element-contents is not a good match for the
way my iCalendar parser works: a list of child nodes is always built up
*before* calling ical:make-ast-node/org-element-create. Sometimes this
list is empty, and sometimes it isn't, but it is always a list, and I
always have a variable bound to this list which I want to provide as an
argument to the constructor. I imagine other parsers often work
similarly. Thus, it would be nice to have an interface where children is
not a &rest argument, so that calling it like

(org-element-create* 'foo (list ...) the-child-nodes)

will work even if the-child-nodes is nil, rather than having to either
(a) say

(if the-child-nodes
    (org-element-create* 'foo (list ...) the-child-nodes)
  (org-element-create* 'foo (list ...)))

or (b) write a wrapper for org-element-create with a different argument
list definition, as I have now.

Is that clearer?

Best,
Richard