JV <[email protected]> writes:
>>> One can't depend on match groups in the current version: group numbering
>>> differs based on which way the pattern matches. This is the reason we're
>>> patching this function.
>>
>> Fair. But then we also need to document the changes in ORG-NEWS.
>
> Done.
I thought about this more, and I am not sure. I still feel that your
change is too breaking.
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.
> +*** Groups are now defined for =org-item-re=
> +
> +Previously, regexp groups returned by =org-item-re= and
> +=org-item-beginning-re= were not predictable and not documented for
> +these functions. Now, calling these functions with the optional
> +~capture-bullet~ argument returns a predictable capture group for the
> +bullet. Omitting this argument returns a regexp without capture
> +groups, and which has faster matching performance.
> ...
> + (let* ((counter `(regexp ,(if org-list-allow-alphabetical
> + "[0-9]+\\|[A-Za-z]"
> + "[0-9]+")))
> + (term (or (car (memq org-plain-list-ordered-item-terminator
> '(?\) ?.)))
> + '(any ".)")))
> + (bullet `(or (any "-+") (seq ,counter ,term)))
> + (ws '(any " \t"))
> + (re (if capture-bullet
> + (rx-to-string ; captures bullet as group 1
> + `(or (seq (0+ ,ws) (group-n 1 ,bullet (or ,ws eol)))
> + (seq (1+ ,ws) (group-n 1 "*" (or ,ws eol)))))
> + (concat (rx-to-string ; minimizes groups (4 vs 7)
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.
I think we should use my earlier idea and keep the group numbering
intact without optional argument.
> * 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.
> +(defun org-in-block-p (&optional types inside element)
> + "Return t when point is in a block element.
> +
> +Block TYPES may be constrained using a type symbol, a name string,
> +or a list including either. Name strings are mapped to type symbols
> +for defined block types and compared as a `special-block' :type
> +property otherwise.
> +
> +When INSIDE is non-nil, return t only when point is between #+BEGIN
> +and #+END lines.
>
> Note that affiliated keywords and blank lines after are considered a
> part of a source block.
>
> When ELEMENT is provided, it is considered to be element at point."
The existing version of org-in-block-p will "Return first block name
matched, or nil". You are changing the return value. Why?
> +(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.
> +(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.
> + (or (and (memq type '(item plain-list)) 'plain-list)
> + (and (memq type org-list--forbidden-block-types)
> + (org-in-block-p type t node))))
So, what happens if we have, say, an example block inside a list?
How will it look like during fontification?
> +(defun org-fontify-extend-space (beg end _old-len)
> + "Extend the region to refontify for plain list indentation and line
> endings.
> +
> +If an outermost plain list is either created or ended by a change, it
> +alters whether indentation is significant for child contents. To
> +accommodate, extend the region to include any indented lines between
> +the end of the change and where either the next list item begins or
> +an outermost list would necessarily end (a non-indented line or the
> +end of buffer).
> +
> +Always extend the region forward to the next line. Even when not
> +needed for plain list indentation, this ensures that end of line
> +fontification is correct for faces with :extend t. This digests edge
> +case fixes from d74a82448 and 1abff3859.
> +See `font-lock-extend-jit-lock-region-after-change' and bug#68849."
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]>
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
