emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: "Gustav Wikström" <gustav@whil.se>
Cc: Carsten Dominik <dominik@uva.nl>,
	"emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>
Subject: Re: [RFC] Org document concept + document property drawers
Date: Sun, 01 Sep 2019 12:38:11 +0200	[thread overview]
Message-ID: <87o904qp2k.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <HE1PR02MB30335B0720ABF7B9780A305FDABC0@HE1PR02MB3033.eurprd02.prod.outlook.com> ("Gustav Wikström"'s message of "Sat, 31 Aug 2019 18:49:42 +0000")

Hello,

Gustav Wikström <gustav@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

  parent reply	other threads:[~2019-09-01 10:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-31 18:49 [RFC] Org document concept + document property drawers Gustav Wikström
2019-09-01  5:20 ` Adam Porter
2019-09-01 10:38 ` Nicolas Goaziou [this message]
2019-09-01 14:41   ` Gustav Wikström
2019-09-06 20:09     ` Nicolas Goaziou
2019-09-29  9:39       ` Gustav Wikström
2019-09-01 15:25 ` Gustav Wikström
2019-09-01 16:11   ` Adam Porter
  -- strict thread matches above, loose matches on Subject: below --
2019-09-01 16:08 Gustav Wikström
2019-09-01 16:26 ` Adam Porter
2019-09-01 18:51 Gustav Wikström
2019-09-01 19:12 Gustav Wikström
2019-09-01 19:17 Gustav Wikström

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o904qp2k.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=dominik@uva.nl \
    --cc=emacs-orgmode@gnu.org \
    --cc=gustav@whil.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).