Re: (Feature Request) have org-edit-special work inside non-environment LaTeX blocks, i.e. \( \) and \[ \]

[Timothy <tecosaur@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3366 bytes --]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Thank you. It looks fine, I will only be nitpicking.

Nitpick away :D

>> +(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.

I had a look at that, to me this was cleaner than using multiple let
bindings, like so

(let ((context ...))
  (unless  ... user error)
  (let* ((contents ...)
         (delim-length ...))
   ...

vs.

(let* ((context ...)
       (_ (unless ... user error))
       (contents ...)
       (delim-length ...))
 ...
       
Personally I find the second one nicer. Thoughts?

>> +    ;; Grab the LaTeX fragment for propertization
> 
> Missing full stop at the end of the comment.

Fixed!

> 
>> +	 (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

I've changed to (string-match-p "\\`\\$[^$]" contents), as this seems
like the idiomatic form, let me know if you're happy with this.

I'm not actually sure what's going on with your second suggested form,
or why that may be better. If you'd mind explaining, that would be
appriciated :)
 
> Missing initial capital and final full stop.

Fixed!

> You could factor out (length contents) so it is only called once.

I'm not sure if this a big deal, but I shoved it in the let* for now,
let me know if that suffices.

>> +    (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.

I'm not quite sure what I should see? I don't notice anything to factor
out here.

> 
>> +       (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.

Added ", so remove newlines"

> Don't leave parenthesis alone.

Fixed!

> Also, make sure your indentation is right, e.g., using M-q on the
> definition.

I've applied auto-indent to `org-edit-latex-fragment'

> You also need to add a proper commit message and use `git format-patch',
> and an entry in ORG-NEWS (probably in Miscellaneous part).

I recall being asked to list modified/added functions, what else do I need?

> Bonus points if you can add some tests in
> "testing/lisp/test-org-src.el".

I'll have a look at that, but I'm not quite sure what to do.

> Could you remind me if you signed the FSF papers already?

They're done and dusted :)

Regards,

Timothy.

[-- Attachment #2: 0001-Extend-org-edit-special-to-editing-LaTeX-fragments.patch --]
[-- Type: application/octet-stream, Size: 3689 bytes --]

From bb494ebfd541343d3f42c312a2efcba5e3cac543 Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Sun, 24 May 2020 23:35:33 +0800
Subject: [PATCH] Extend org-edit-special to editing LaTeX-fragments Defines a
 new function, `org-edit-latex-fragment' which is hooked into
 `org-edit-special', modifying `org-src--contents-area-modified' to recognise
 the element type.

---
 lisp/org-src.el | 45 +++++++++++++++++++++++++++++++++++++++++++++
 lisp/org.el     |  1 +
 2 files changed, 46 insertions(+)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 1643607e4..ebc443a16 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -363,6 +363,12 @@ where BEG and END are buffer positions and CONTENTS is a string."
 	     (end (progn (goto-char (org-element-property :end datum))
 			 (search-backward "}" (line-beginning-position) t))))
 	 (list beg end (buffer-substring-no-properties beg end))))
+      ((eq type 'latex-fragment)
+       (let ((beg (org-element-property :begin datum))
+	     (end (org-with-point-at (org-element-property :end datum)
+		    (skip-chars-backward " \t")
+		    (point))))
+	 (list beg end (buffer-substring-no-properties beg end))))
       ((org-element-property :contents-begin datum)
        (let ((beg (org-element-property :contents-begin datum))
 	     (end (org-element-property :contents-end datum)))
@@ -959,6 +965,45 @@ Throw an error when not at such a table."
     (table-recognize)
     t))
 
+(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")))
+	 ;; Grab the LaTeX fragment for propertization.
+	 (contents (buffer-substring-no-properties
+		    (org-element-property :begin context)
+		    (- (org-element-property :end context)
+		       (org-element-property :post-blank context))))
+	 (contents-length (length contents))
+	 (delim-length (if (string-match-p "\\`\\$[^$]" contents))
+		       1 2))
+    ;; Make the LaTeX deliminators read-only.
+    (add-text-properties
+     0 delim-length
+     '(read-only "Cannot edit LaTeX deliminator" front-sticky t rear-nonsticky t)
+     contents)
+    (add-text-properties
+     (- contents-length delim-length)
+     contents-length
+     '(read-only "Cannot edit LaTeX deliminator" front-sticky nil rear-nonsticky nil)
+     contents)
+    (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
+       (while (re-search-forward "\n[ \t]*\n" nil t) (replace-match "\n"))
+       ;; If within a table a newline would disrupt the structure, so remove newlines
+       (goto-char (point-min))
+       (when (org-element-lineage context '(table-cell))
+	 (while (search-forward "\n" nil t) (replace-match " "))))
+     contents)
+    t))
+
 (defun org-edit-latex-environment ()
   "Edit LaTeX environment at point.
 \\<org-src-mode-map>
diff --git a/lisp/org.el b/lisp/org.el
index 40c3c46b9..0808fc210 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -17347,6 +17347,7 @@ Otherwise, return a user error."
 	 (pcase (org-element-type context)
 	   (`footnote-reference (org-edit-footnote-reference))
 	   (`inline-src-block (org-edit-inline-src-code))
+	   (`latex-fragment (org-edit-latex-fragment))
 	   (`timestamp (if (eq 'inactive (org-element-property :type context))
 			   (call-interactively #'org-time-stamp-inactive)
 			 (call-interactively #'org-time-stamp)))
-- 
2.26.2