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

[Nicolas Goaziou]

Hello,

Gustav Wikström <address@hidden> writes:

> 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.

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.

> 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.

> ** (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.

> ** (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".

> +;; 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.

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

Ditto.

> +(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'.

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

Non-nil if ...

> +  (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.

Also, you could use `org-match-line' in this case.


Regards,

-- 
Nicolas Goaziou