Ihor Radchenko wrote:
Before we go too far, I not that you do not appear to have FSF copyright
assignment. For non-trivial patches we need to have the paperwork with
FSF. Would you consider completing it? See
https://orgmode.org/worg/org-contribute.html#copyright
I submitted a request for the paperwork.
Try opening the attached file and scrolling.
The performance is severely degraded with your patch.
I compared performance with many indented list items to other repeated
fontified elements. The previous patch version takes 2.5x as long in
font-lock-fontify-region for indented list items as headlines. I've been
able to get this down to 1.5x.
The most expensive calls are ones to the org-element API.
org-element-at-point and org-element-lineage are the main contributors.
In a tight loop, they add up. Even replacing org-element-type with 'car
yields a measurable improvement. And GC is definitely a factor.
If you have a particular performance standard you think this should
meet, please let me know. Otherwise I will squeeze what's available,
benchmark it, and if it feels responsive in the extreme test case, send
it for your review.
We should not remove existing groups.
Not because it is a good thing to have them, but because third-party code
may rely on the current behavior.
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.
I also couldn't find any measurable difference between capturing and
non-capturing regexp groups at any scale.
This is simply a rule of thumb from the maintainer of Emacs regexp
matcher.
And helpfully, when groups are non-capturing, we have no worry about
third-party code depending on them.
When ELEMENT is provided, it is considered to be element at point."
(save-match-data (setq element (or element (org-element-at-point))))
- (when (org-element-type-p element 'src-block)
+ (when-let* ((types (or (org--block-types types) org-element-block-elements))
+ (element (or (org-element-lineage element types t)
+ (org-element-lineage-map element
+ `(let ((type (org-element-property :type node)))
+ (when (member type ',types) node))
+ 'special-block t t))))
Why only 'special-block?
It's the only case in which element type doesn't match the #+begin_ and
#+end_ declarations. The pre-patch definition of org-in-block-p looks at
the begin/end lines, so this preserves its behavior.
I should note that the patch to org-in-block-p will likely change for
the performance reasons discussed above. I imagined a second call to
org-element-lineage/map would hit the cache and be inexpensive, but the
profiling data suggests otherwise.
For new faces and features, we need an entry in ORG-NEWS.
Will do.
Also, :package-version should be added to defface I think.
What package version should I list?
You change this new function in the next patch. Can squash the changes here.
Will do on this and your other comments. All appreciated.
