Hello, Gustav Wikström <gus...@whil.se> 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