Ihor Radchenko wrote:

I thought about this more, and I am not sure. I still feel that your
change is too breaking.

Ok, we can do something that leaves the current regexp intact.

I looked at org-item-re again, and my understanding is that group
*numbering* is always the same. It is just that some groups may be
nil. But that's what the existing code must handle as of now.

I further looked into the callers, and I note that `org-at-item-p'
has
;; Set match data.
      (looking-at (org-item-re))

which is very explicit.

Yes, but only match group 0 is ever referenced. I did check this.

This will not be 4 vs 7 groups. This will be 0 or 1 groups.
I tried your patch and did (insert (org-item-re)). I got
\(?:[    ]*\(?:[+-]\|\(?:[0-9]+\)[).]\)\|[       ]+\*\)\(?:[     ]\|$\)
No capturing groups present.

It is 0 vs 1 capturing groups, but 4 vs 7 groups. The distinction here is for performance, and since there is no measurable performance difference between capturing and non-capturing groups, 4 and 7 should be the relevant counts. At least this is what my performance analysis suggested.

I think we should use my earlier idea and keep the group numbering
intact without optional argument.

This may be simpler than I thought. It seems checkbox fontification already uses org-list-full-item-re. We can just add another subexp highlighter to this. A faster regexp would be nice, but it only makes sense if it could completely replace the slower one for font locking.

* lisp/org-element.el (org-element-block-elements): Add new constant.

I do not like having a whole new constant. We will need to keep it
updated if new block types are added. I'd rather just match against
*-block type pattern. At least, we should compute the value dynamically.

I don't love constant proliferation either, but I do think org-element needs some canonical way to answer "what elements are blocks?" or at least "is this element a block?" We can certainly compute the value dynamically as you suggest, as long as it is precomputed. At runtime, we want type comparisons to be as cheap as possible.

The existing version of org-in-block-p will "Return first block name
matched, or nil". You are changing the return value. Why?

There are two functions that I consolidated together: org-in-block-p and org-in-src-block-p. The latter was more capable, so I retained most of it including its return value. The earlier version of org-in-block-p is summarized as "Non-nil when point belongs to a block whose name belongs to NAMES"; the function is named using the convention for boolean predicate functions; and there are no non-boolean uses within the org code base (it's only used twice). If you agree that this is ok, perhaps the return type merits mentioning in ORG-NEWS?

+(defconst org-list--forbidden-block-types
+  '(example-block export-block src-block verse-block)
+  "Types of blocks where lists are not allowed.")

This is awkward as we will need to maintain this constant.
You can instead check org-element-restriction.
For example, you can bind this variable to a dynamically computed list
that examines org-element-restriction.

org-element-restriction returns allowed objects but not allowed elements, correct? If so, we need something else.

Could we simply use org-element-greater-elements? Specifically, I am thinking that org-list-forbidden-blocks should be the org-element-block-elements that are not in org-element-greater-elements. Does this sound correct?

Currently, there is one exception to this: comment-block. This type is not in the existing org-list-forbidden-blocks list, but it is also not in org-element-greater-elements. What is the proper treatment of org syntax within comment blocks? Should it be fontified or not?

+(defun org-in-list-context-p (element)
+  "Return t if in a plain list and not inside a list-forbidding block.
+This identifies whether indentation is meaningful syntax for ELEMENT."
+  ;; This function should maintain performance suitable for frequently
+  ;; repeated calls by fontify machinery.  Accordingly, it minimizes
+  ;; traversal, uses car to get element type, and calls org-in-block-p
+  ;; only if needed after a faster type check.
+  (eq 'plain-list
+      (org-element-lineage-map element
+          `(let ((type (car node)))

Sorry, but this is fragile. Let's not use unsafe code practices, even if
we want performance optimization. This will backfire sooner or later.

Yes, you're right. That's wise.

So, what happens if we have, say, an example block inside a list?
How will it look like during fontification?

In this case, the contents inside the example block will not be fontified as a list. The #+begin, #+end, and affiliated lines will be.

Then, why not doing what Stefan suggested there:

      I still hope you can change Org mode so it uses
      `font-lock-extend-region-functions` instead of
      `font-lock-extend-after-change-region-function`.
                  Stefan Monnier <[email protected]>


This does seem like a good opportunity to make that change. Will do.





Reply via email to