emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Timothy <tecosaur@gmail.com>
To: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Cc: Bastien <bzg@gnu.org>, "emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>
Subject: Re: (Feature Request) have org-edit-special work inside non-environment LaTeX blocks, i.e. \( \) and \[ \]
Date: Tue, 26 May 2020 16:39:41 +0800	[thread overview]
Message-ID: <BDF96401-83EB-4F79-BD8A-BADC0262FE06@getmailspring.com> (raw)
In-Reply-To: <87wo4z6sec.fsf@nicolasgoaziou.fr>

[-- 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


  reply	other threads:[~2020-05-26  8:40 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-19  1:31 (Feature Request) have org-edit-special work inside non-environment LaTeX blocks, i.e. \( \) and \[ \] Timothy
2020-05-19  7:47 ` Nicolas Goaziou
2020-05-19  9:27   ` Timothy
2020-05-19  9:39     ` Nicolas Goaziou
2020-05-19  9:45       ` Timothy
2020-05-19 13:27         ` Nicolas Goaziou
2020-05-19 13:32           ` Timothy
2020-05-19 14:09             ` Nicolas Goaziou
2020-05-19 14:12               ` Timothy
2020-05-19 14:28                 ` Nicolas Goaziou
2020-05-19 14:33                   ` Timothy
2020-05-23  9:10 ` Bastien
2020-05-23  9:20   ` tecosaur
2020-05-23  9:34     ` Bastien
2020-05-24 15:07   ` TEC
2020-05-24 15:38     ` TEC
2020-05-24 15:43       ` Timothy
2020-05-24 19:33         ` Nicolas Goaziou
2020-05-25  1:33           ` TEC
2020-05-25  7:11             ` Nicolas Goaziou
2020-05-25  9:28           ` TEC
2020-05-25  9:41             ` Nicolas Goaziou
2020-05-25  9:42               ` TEC
2020-05-25  9:55                 ` TEC
2020-05-25 10:09                   ` Nicolas Goaziou
2020-05-25 10:09                     ` TEC
2020-05-25 10:23                       ` Nicolas Goaziou
2020-05-25 10:24                         ` TEC
2020-05-25 10:32                           ` Nicolas Goaziou
2020-05-25 10:33                             ` TEC
2020-05-25 10:05                 ` TEC
2020-05-25 10:20                   ` Nicolas Goaziou
2020-05-25 10:21                     ` TEC
2020-05-25 11:27                       ` Nicolas Goaziou
2020-05-25 13:22                         ` Timothy
2020-05-25 14:08                           ` TEC
2020-05-26  7:56                             ` Nicolas Goaziou
2020-05-26  8:39                               ` Timothy [this message]
2020-05-26  9:11                                 ` Nicolas Goaziou
2020-05-26  9:23                                   ` TEC
2020-05-26 13:56                                     ` TEC
2020-05-26 21:13                                       ` Nicolas Goaziou
2020-05-26 17:01                                 ` John Kitchin
2020-05-25 18:58                           ` Nicolas Goaziou
2020-05-26  4:48                             ` TEC
2020-05-25 10:11                 ` Nicolas Goaziou
2020-05-25 10:17                   ` TEC
  -- strict thread matches above, loose matches on Subject: below --
2020-05-18  6:44 Timothy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BDF96401-83EB-4F79-BD8A-BADC0262FE06@getmailspring.com \
    --to=tecosaur@gmail.com \
    --cc=bzg@gnu.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=mail@nicolasgoaziou.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).