Re: [PATCH] org-element: Hide parsers boilerplate into plist-creating macros
Hello, Bastien writes: > akater writes: > >> We replace some repetetive code with macro calls org-prog-plist and >> org-let*-prog-plist. > > IIUC this is a refactoring, it does not add or remove functionalities? > > I'll let Nicolas decide on this, of course. FWIW, I'm not convinced there's a significant advantage in doing this refactoring. Arguably, the code is not more readable. It will not make it faster either. There is room for refactoring in Org (for the record, org-agenda.el has not switched to lexical binding yet), but I don't think org-element.el has bitrotten so much that it deserves some. Regards, -- Nicolas Goaziou
Re: [PATCH] org-element: Hide parsers boilerplate into plist-creating macros
akater writes: > Bastien writes: > >> IIUC this is a refactoring, it does not add or remove functionalities? > > Yes, just a refactoring. In any case, let's discuss this for after 9.4. Thanks, -- Bastien
Re: [PATCH] org-element: Hide parsers boilerplate into plist-creating macros
Bastien writes: > IIUC this is a refactoring, it does not add or remove functionalities? Yes, just a refactoring. A typo crept into comments and help pages, so here's an update. signature.asc Description: PGP signature >From 0636f032b3c7e60221c28aeea91cce58376561dd Mon Sep 17 00:00:00 2001 From: akater Date: Thu, 16 Apr 2020 02:25:59 + Subject: [PATCH] org-element: Hide parsers boilerplate into plist-creating macros * lisp/org-element.el (org-prog-plist, org-let*-prog-plists) (org-let*-prog-plist): New macros. Build plists without boilerplate. (org-fold, org-dekeyword): New functions. Dependencies for the above. * lisp/org-element.el (org-element-center-block-parser) (org-element-drawer-parser, org-element-dynamic-block-parser) (org-element-footnote-definition-parser) (org-element-plain-list-parser, org-element-property-drawer-parser) (org-element-quote-block-parser, org-element-section-parser) (org-element-special-block-parser, org-element-babel-call-parser) (org-element-clock-parser, org-element-comment-parser) (org-element-comment-block-parser, org-element-diary-sexp-parser) (org-element-example-block-parser, org-element-export-block-parser) (org-element-fixed-width-parser, org-element-horizontal-rule-parser) (org-element-keyword-parser, org-element-latex-environment-parser) (org-element-node-property-parser, org-element-paragraph-parser) (org-element-planning-parser, org-element-src-block-parser) (org-element-table-parser, org-element-table-row-parser) (org-element-verse-block-parser, org-element-entity-parser) (org-element-footnote-reference-parser, org-element-inline-babel-call-parser) (org-element-inline-src-block-parser, org-element-latex-fragment-parser) (org-element-link-parser): Use org-prog-plist to build plist (org-element-headline-parser, org-element-inlinetask-parser) (org-element-item-parser, org-element-timestamp-parser): Use org-let*-prog-plist to build plist (org-element-radio-target-parser, org-element-statistics-cookie-parser) (org-element-subscript-parser, org-element-superscript-parser) (org-element-table-cell-parser, org-element-target-parser) (org-element-underline-parser, org-element-verbatim-parser): Use just #'list to build plist (org-element-comment-block-parser): Fix a typo in docstring. --- lisp/org-element.el | 2584 +++ lisp/org-macs.el| 259 + 2 files changed, 1421 insertions(+), 1422 deletions(-) diff --git a/lisp/org-element.el b/lisp/org-element.el index a693cb68d..e40f881b9 100644 --- a/lisp/org-element.el +++ b/lisp/org-element.el @@ -691,29 +691,26 @@ Assume point is at the beginning of the block." (re-search-forward "^[ \t]*#\\+END_CENTER[ \t]*$" limit t))) ;; Incomplete block: parse it as a paragraph. (org-element-paragraph-parser limit affiliated) - (let ((block-end-line (match-beginning 0))) - (let* ((begin (car affiliated)) - (post-affiliated (point)) + (list 'center-block + (nconc + (org-prog-plist + block-end-line (match-beginning 0) + :begin (car affiliated) + :post-affiliated (point) ;; Empty blocks have no contents. - (contents-begin (progn (forward-line) + :contents-begin (progn (forward-line) (and (< (point) block-end-line) - (point - (contents-end (and contents-begin block-end-line)) - (pos-before-blank (progn (goto-char block-end-line) - (forward-line) - (point))) - (end (save-excursion + (point))) + :contents-end (and contents-begin block-end-line) + pos-before-blank (progn (goto-char block-end-line) + (forward-line) + (point)) + :end (save-excursion (skip-chars-forward " \r\t\n" limit) - (if (eobp) (point) (line-beginning-position) - (list 'center-block - (nconc - (list :begin begin - :end end - :contents-begin contents-begin - :contents-end contents-end - :post-blank (count-lines pos-before-blank end) - :post-affiliated post-affiliated) - (cdr affiliated + (if (eobp) (point) + (line-beginning-position))) + :post-blank (count-lines pos-before-blank end)) + (cdr affiliated)) (defun org-element-center-block-interpreter (_ contents) "Interpret a center-block element as Org syntax. @@ -740,32 +737,28 @@ Assume point is at beginning of drawer." (if (not (save-excursion (re-search-forward "^[ \t]*:END:[ \t]*$" limit t))) ;; Incomplete drawer: parse it as a paragraph. (org-element-paragraph-parser limit affiliated) - (save-excursion - (let* ((drawer-end-line (match-beginning 0)) - (name (progn (looking-at org-drawer-regexp) - (match-string-no-properties 1))) - (begin (car affiliated)) - (post-affiliated (point)) - ;; Empty drawers have no contents. - (contents-begin (progn (forward-line) -
Re: [PATCH] org-element: Hide parsers boilerplate into plist-creating macros
Hi Akater, akater writes: > We replace some repetetive code with macro calls org-prog-plist and > org-let*-prog-plist. IIUC this is a refactoring, it does not add or remove functionalities? I'll let Nicolas decide on this, of course. -- Bastien
[PATCH] org-element: Hide parsers boilerplate into plist-creating macros
We replace some repetetive code with macro calls org-prog-plist and org-let*-prog-plist. The macros are not very conventional but hopefully their docstrings are illustrative enough. In effect, all subexpressions of the form :begin begin :end end :contents-begin contents-begin :contents-end contents-end and so on, are removed, together with some let* forms. Macros expand to code that is essentially the original code, only the order of key-value pairs in resulting plists is different. One might argue that it is desirable to have key-value pairs plisted in specific order, maybe somewhat unified. A rejoinder: plists are meant to be order-independent while those who delve into these fairly low-level plists regularly enough to be bothered by the properties' order, can be considered org-element experts (voluntary or not); I believe it is only instructive to an expert to be reminded of the structure of the algorithm that constructs plist in question, especially if such algorithms are highly imperative. That said, I did rearrange some assignments to make resulting plists look a little prettier. I also outlined (but not implemented) a mechanism for (partially) specifying positions, in comments. I tested most redefined parsers with new definitions applying them to one sample object of each kind. Left untested (as I'm not familiar with Org markup for the corresponding objects) are - inlinetask-parser - diary-sexp-parser - horizontal-rule-parser - planning-parser - entity-parser - export-snippet-parser - latex-fragment-parser - macro-parser - radio-target-parser - statistics-cookie-parser - target-parser Still, diff shows that only trivial subexpressions, as described above, are discarded there. I did check that Org(+contrib) builds with this patch. Minor note on org-element-inline-babel-call-parser: org-element--parse-paired-brackets alters point. That's why I felt it would be more appropriate to put the corresponding binding/assignment on top level of an explicitly imperative macro, rather than keep the binding in a more localized let form, as extent of the operation is not localized. signature.asc Description: PGP signature >From d9d108f97917c1b55841df907510bcc89f8db406 Mon Sep 17 00:00:00 2001 From: akater Date: Thu, 16 Apr 2020 02:25:59 + Subject: [PATCH] org-element: Hide parsers boilerplate into plist-creating macros * lisp/org-element.el (org-prog-plist, org-let*-prog-plists) (org-let*-prog-plist): New macros. Build plists without boilerplate. (org-fold, org-dekeyword): New functions. Dependencies for the above. * lisp/org-element.el (org-element-center-block-parser) (org-element-drawer-parser, org-element-dynamic-block-parser) (org-element-footnote-definition-parser) (org-element-plain-list-parser, org-element-property-drawer-parser) (org-element-quote-block-parser, org-element-section-parser) (org-element-special-block-parser, org-element-babel-call-parser) (org-element-clock-parser, org-element-comment-parser) (org-element-comment-block-parser, org-element-diary-sexp-parser) (org-element-example-block-parser, org-element-export-block-parser) (org-element-fixed-width-parser, org-element-horizontal-rule-parser) (org-element-keyword-parser, org-element-latex-environment-parser) (org-element-node-property-parser, org-element-paragraph-parser) (org-element-planning-parser, org-element-src-block-parser) (org-element-table-parser, org-element-table-row-parser) (org-element-verse-block-parser, org-element-entity-parser) (org-element-footnote-reference-parser, org-element-inline-babel-call-parser) (org-element-inline-src-block-parser, org-element-latex-fragment-parser) (org-element-link-parser): Use org-prog-plist to build plist (org-element-headline-parser, org-element-inlinetask-parser) (org-element-item-parser, org-element-timestamp-parser): Use org-let*-prog-plist to build plist (org-element-radio-target-parser, org-element-statistics-cookie-parser) (org-element-subscript-parser, org-element-superscript-parser) (org-element-table-cell-parser, org-element-target-parser) (org-element-underline-parser, org-element-verbatim-parser): Use just #'list to build plist (org-element-comment-block-parser): Fix a typo in docstring. --- lisp/org-element.el | 2584 +++ lisp/org-macs.el| 259 + 2 files changed, 1421 insertions(+), 1422 deletions(-) diff --git a/lisp/org-element.el b/lisp/org-element.el index a693cb68d..e40f881b9 100644 --- a/lisp/org-element.el +++ b/lisp/org-element.el @@ -691,29 +691,26 @@ Assume point is at the beginning of the block." (re-search-forward "^[ \t]*#\\+END_CENTER[ \t]*$" limit t))) ;; Incomplete block: parse it as a paragraph. (org-element-paragraph-parser limit affiliated) - (let ((block-end-line (match-beginning 0))) - (let* ((begin (car affiliated)) - (post-affiliated (point)) + (list 'center-block + (nconc + (org-prog-plist + block-end-line
