Hello,
TEC <tecos...@gmail.com> writes:
> ---
> lisp/org-src.el | 45 +++++++++++++++++++++++++++++++++++++++++++++
> lisp/org.el | 1 +
> 2 files changed, 46 insertions(+)
Thank you. It looks fine, I will only be nitpicking.
> +(defun org-edit-latex-fragment ()
> + "Edit LaTeX fragment at point."
> + (interactive)
> + (let* ((context (org-element-context))
> + (_ (unless (and (eq (org-element-type context) 'latex-fragment)
> + (org-src--on-datum-p context))
> + (user-error "Not on a LaTeX fragment")))
This is a fancy way to use a let-binding. I suggest to mimic what is
done elsewhere, i.e., first bind `context', then check if we're at
a LaTeX fragment, then bind the rest.
> + ;; Grab the LaTeX fragment for propertization
Missing full stop at the end of the comment.
> + (contents (buffer-substring-no-properties
> + (org-element-property :begin context)
> + (- (org-element-property :end context)
> + (org-element-property :post-blank context))))
> + (delim-length (if (string-match "\\$[^$]" (substring contents 0 2))
Use
(string-match "\\`\\$[^$]" contents)
instead.
or, arguably better,
(string-match (rx (seq string-start "$" (not (any "$"))))
contents)
> + 1 2)))
> + ;; make the LaTeX deliminators read-only
Missing initial capital and final full stop.
> + (add-text-properties
> + 0 delim-length
> + '(read-only "Cannot edit LaTeX deliminator" front-sticky t
> rear-nonsticky t)
> + contents)
> + (add-text-properties
> + (- (length contents) delim-length)
> + (length contents)
> + '(read-only "Cannot edit LaTeX deliminator" front-sticky nil
> rear-nonsticky nil)
> + contents)
You could factor out (length contents) so it is only called once.
> + (org-src--edit-element
> + context
> + (org-src--construct-edit-buffer-name (buffer-name) "LaTeX fragment")
> + (org-src-get-lang-mode "latex")
> + (lambda ()
> + ;; Blank lines break things, replace with a single newline
See above.
> + (while (re-search-forward "\n[ \t]*\n" nil t) (replace-match "\n"))
> + ;; If within a table a newline would disrupt the structure,
This comment is truncated.
> + (goto-char (point-min))
> + (when (org-element-lineage context '(table-cell))
> + (while (search-forward "\n" nil t) (replace-match " ")))
> + )
Don't leave parenthesis alone.
Also, make sure your indentation is right, e.g., using M-q on the
definition.
You also need to add a proper commit message and use `git format-patch',
and an entry in ORG-NEWS (probably in Miscellaneous part).
Bonus points if you can add some tests in
"testing/lisp/test-org-src.el".
Could you remind me if you signed the FSF papers already?
Regards,
--
Nicolas Goaziou
