JV <[email protected]> writes:
> Please see the patch set attached.
Thanks, and sorry for the late reply.
I have a handful of comments.
> All tests pass against main prior to the recent introduction of a bug in
> org-colview function org-columns--line-face that doesn't handle multiple
> faces on headlines. I submitted a bug report for this separately.
Note that string-equal-ignore-case is not available in Emacs 28.
> * lisp/org-list.el (org-item-re): Update generated regexp so the
> bullet is always matched as group 1, independently of leading
> whitespace.
It is OK to provide a group in regexp, but adding groups make the
matching much slower. So, let's not modify the default regexp. Instead,
I recommend adding an optional parameter to org-item-re that will make
it return a version of regexp with groups.
> Subject: [PATCH 2/3] Improve org-in-block-p
> ...
> +(defconst org-element-block-elements
> + '(center-block comment-block dynamic-block example-block
> + export-block quote-block special-block src-block
> verse-block)
> + "List of block element types.")
What about simply matching the element type against ^.*-block$?
Here is an example:
(defalias 'org-in-block-p #'org-at-block-p)
(defun org-at-block-p (&optional names)
"Return block name, as a string, if cursor is at a block.
When optional argument NAMES is non-nil, it should be a list of block
names as strings."
(when names
(setq names
(mapcar
(lambda (name) (intern (format "%s-block" (downcase name))))
names)))
(let ((element (org-element-at-point)))
(when
(org-element-type-p
element
(or names
'( center-block comment-block dynamic-block
example-block export-block quote-block
special-block src-block verse-block)))
(string-match "\\([^-]+\\)-block" (symbol-name (org-element-type
element)))
(match-string 1 (symbol-name (org-element-type element))))))
> +(defconst org-element-block-element-names
> + '(("CENTER" . center-block)
> + ("COMMENT" . comment-block)
> + ("EXAMPLE" . example-block)
> + ("EXPORT" . export-block)
> + ("QUOTE" . quote-block)
> + ("SRC" . src-block)
> + ("VERSE" . verse-block))
> + "Alist of block names to element types.")
If we can match against the type, this will no longer be needed.
> +(defun org--block-types (types)
> + "Convert block names to types and filter non-strings to block types.
> +Name strings that do not map to a defined block type are returned unchanged."
> + (mapcar (lambda (type)
> + (if (stringp type)
> + (alist-get type org-element-block-element-names
> + type nil #'string-equal-ignore-case)
> + (car (memq type org-element-block-elements))))
> + (ensure-list types)))
Same here.
> +(defvar-local org-list-outermost-end nil
> + "The end position of the outermost plain list around point, or nil.")
We should generally avoid dynamic variables in the code, if possible.
> (defun org-before-change-function (_beg _end)
> - "Every change indicates that a table might need an update."
> + "Every change indicates that a plain list or table might need an update."
> + (setq org-list-outermost-end (org-element-end (org-list-outermost)))
This will fire the parser before every single change.
Sorry, but this will slow down Org too much. We need to find some other
way. We just got a report that even the existing approach is already too
slow. With your addition, Org performance will crawl.
With the example file provided in
https://orgmode.org/list/87y0g9hx38.fsf@localhost, your patch will make
typing impossible.
--
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>
