Re: [O] [RFC] Org document concept + document property drawers

[Gustav Wikström]

Hi Nicolas,

> -----Original Message-----
> From: Nicolas Goaziou <address@hidden>
> 
> > I'm continuing on my proposal to introduce a "document" element in
> > org-mode and the idea of seeing everything before the first headline
> > as the base level 0 outline for a file. I've attached two patches that
> > I'd like some public review of before pushing to master.
> 
> I will not review fully the patches, as I have no time for that.
> However, I will make a few comments about it.
> 
> First, you should show a few examples of what an Org document would look
> like, compared to what we have already, focusing particularly on the
> advantages, and what is now invalid. It is a good thing to do if you
> expect comments, as you cannot ask everyone to eyeball through the whole
> patch set.

Sure, I'll do that as a reply to my own mail to not make that comment
disappear among the details here.

> Also, whatever the outcome of the discussion is, /nothing should go in
> master as long as Org 9.3 is not released/. This looks like a breaking
> change at the most lower level (syntax, parser...), I think it may
> trigger a new major release.

I'm starting out slow by making this a non-breaking change. At least
I'm trying to. I.e. everything that worked before should continue to
work.

> > Patch 0001 introduces the document element into org-element.el, and
> > some restructuring related to that.
> 
> This should be explained in comments, and, if it lands at some point,
> Worg pages about syntax and exporter should be updated, too.

Which comments do you mean? I've tried to be rigorous in the patch
notes. But you mean in the mail itself?

> 
> > ** (renamed, modified) org--setup-collect-keywords -> org-collect-keywords
> > Renamed and generalized org--setup-collect-keywords to make it work
> > for multiple purposes.  Is not limited to a fixed set of keywords any
> > longer.  New name: org-collect-keywords.
> 
> An important typo note: we use "Org mode", or "an Org document", not
> "Org-mode" and "an org-document". Hyphens are only used to refer
> explicitly to a Lisp symbol, or its value or function.

Ah of course. I keep forgetting that one.

> 
> > ** (modified) org-element-keyword-parser
> > Uses (new) org-keyword-regexp instead of hardcoding it's own regexp.
> 
> Keep in mind that Org Element library should ultimately be as
> independent as possible to the other parts of Org, including "org.el".

Got it - is it preferred to do it the other way around - I.e. define
the regexp in org-element.el rather than org.el if there is use of the
regexp in both files?

> > +;; Org-element can parse org-mode documents.  The top-node in the
> > +;; parse-tree will always have TYPE `org-data' and PROPERTIES nil.
> 
> See my remark about typography above.

Got it.

> 
> > +;; The following part creates a fully recursive org-mode parser.
> 
> Ditto.

Ok, ofc.

> > +(defun org-back-to-heading-or-point-min (&optional invisible-ok)
> > +  "Go back to heading or first point in buffer.
> > +If point is before first heading go to first point in buffer
> > +instead of back to heading."
> > +  (condition-case nil
> > +      (outline-back-to-heading invisible-ok)
> > +    (error
> > +     (goto-char (point-min)))))
> 
> Try to limit use of Outline functions. They are generally slower than
> their Org counterpart. This is not true in this case, but, one day, we
> might optimize `org-back-to-heading'.

Noted, except in this case I think it's warranted since
org-back-to-heading behaves exactly the same with the exception of
raising an error if before the first heading. Since both functions are
defined next to another I'm sure a later refactor will take care of
both since they're structurally identical and defined two lines apart.

> > +(defun org-at-keyword-p nil
> > +  "Is cursor at a keyword-line?"
> 
> Non-nil if ...

I've just "pattern-matched" myself into that docstring. It matches the
existing definitions of org-at-drawer-p, org-at-comment-p,
org-at-block-p. Which are defined around this.

> > +  (save-excursion
> > +    (move-beginning-of-line 1)
> > +    (looking-at org-keyword-regexp)))
> 
> While this is technically correct today, please don't write a predicate
> only based on regexps, use the parser for that. For example, the parser
> can understand
> 
>   #+begin_example
>   #+keyword: not a keyword
>   #+end_example
> 
> whereas your function cannot.

Hmm... I see your point. Have to think a bit here - because I have the
feeling that defining the predicate using org-element-at-point will
incur quite a performance hit. Or what is your intuition regarding
that?

> 
> Also, you could use `org-match-line' in this case.
> 
> 
> Regards,
> 
> --
> Nicolas Goaziou

Thanks for your initial thoughts.

/G