Subject: bug#61400: Different filling for verbatim macros

From: Ikumi Keita <[email-masked]>
Date: Mon, 13 Feb 2023 00:32:06 +0900

Hi Arash and all,

>>>>> Arash Esbati <[email-masked]> writes:

> I think this should be our course of action: Make
> `LaTeX-current-verbatim-macro' and `TeX-current-macro' work identical.

I'm now thinking a different approach. Please see below.

> Here is another idea: We don't use `LaTeX-verbatim-p' in
> `fill-nobreak-predicate', but define two new functions, say
> `LaTeX-verbatim-macro-arg-boundaries' and `LaTeX-verbatim-nobreak-p'.
> The first one is carved out of `LaTeX-verbatim-macro-boundaries' and
> returns the beginning and end positions of the argument, as the name
> suggests.

> And `LaTeX-verbatim-nobreak-p' does the following: Check if the possible
> break point for filling is inside the range returned by
> `LaTeX-verbatim-macro-arg-boundaries' and if
> `LaTeX-current-verbatim-macro' on that point returns non-nil, then
> inhibit break there (t value), otherwise allow brake (nil value.)

> WDYT, does this make sense?

Looking at the current code closely, I think we can take a more drastic
route. :-)

(1) `LaTeX-current-verbatim-macro' is used only in
`LaTeX-verbatim-p':
,----
| > git grep -E LaTeX-current-verbatim-macro
| ChangeLog.1:    (LaTeX-current-verbatim-macro): Include asterisk, if present, in
| ChangeLog.1:    (LaTeX-verbatim-macro-boundaries, LaTeX-current-verbatim-macro)
| latex.el:(defun LaTeX-current-verbatim-macro ()
| latex.el:        (member (LaTeX-current-verbatim-macro)
`----
And `LaTeX-verbatim-macro-boundaries' is used only in
`LaTeX-current-verbatim-macro':
,----
| > git grep -E LaTeX-verbatim-macro-boundaries
| ChangeLog.1:    * latex.el (LaTeX-verbatim-macro-boundaries): Support verbatim
| ChangeLog.1:    * latex.el (LaTeX-verbatim-macro-boundaries): Do step backwards 
| at
| ChangeLog.1:    (LaTeX-verbatim-macro-boundaries): Find macro in case point is
| ChangeLog.1:    (LaTeX-verbatim-macro-boundaries, LaTeX-current-verbatim-macro)
| latex.el:(defun LaTeX-verbatim-macro-boundaries ()
| latex.el:  (let ((macro-boundaries (LaTeX-verbatim-macro-boundaries)))
`----

Those two functions are practically dedicated to `LaTeX-verbatim-p'.
Hence we can be rather flexible about what we do for them.

(2) `LaTeX-verbatim-p' is currently used only in
`fill-nobreak-predicate', `LaTeX-search-forward-comment-start' and
`TeX-in-comment' (through `TeX-verbatim-p-function'):
,----
| > git grep -E LaTeX-verbatim-p
| ChangeLog.1:    (LaTeX-verbatim-p): Doc fix.  Correct macro lookup.
| ChangeLog.1:    * style/verbatim.el (LaTeX-verbatim-package-options): New
| ChangeLog.1:    (LaTeX-verbatim-p, LaTeX-search-forward-comment-start): New
| latex.el:(defun LaTeX-verbatim-p (&optional pos)
| latex.el:                            (not (LaTeX-verbatim-p)))
| latex.el:               #'LaTeX-verbatim-p t)
| latex.el:  (setq TeX-verbatim-p-function #'LaTeX-verbatim-p)
| style/verbatim.el:(defvar LaTeX-verbatim-package-options nil
`----
Thus it's safe to modify `LaTeX-verbatim-p' to return nil at just before
"\verb|...|" etc. by the following reasons:
o In `LaTeX-search-forward-comment-start', `LaTeX-verbatim-p' is always
  called just before % sign.
o In `TeX-in-comment', `LaTeX-verbatim-p' is always called in a comment.

(3) I began to feel unsatisfied that `LaTeX-verbatim-p' needs the check
for `LaTeX-verbatim-macros-with-braces' in addition to
`LaTeX-current-verbatim-macro':
----------------------------------------------------------------------
(defun LaTeX-verbatim-p (&optional pos)
[...]
        (member (LaTeX-current-verbatim-macro)
                (LaTeX-verbatim-macros-with-delims))
        (member (TeX-current-macro) (LaTeX-verbatim-macros-with-braces)) <--- Here
        (member (LaTeX-current-environment) (LaTeX-verbatim-environments)))))
----------------------------------------------------------------------
Why can't `LaTeX-current-verbatim-macro' check both *-delims and
*-braces at the same time? Its name suggests it _should_ be able to
handle either type.

Looking at the contents of `LaTeX-verbatim-macro-boundaries', I came to
think that this is due to insufficient implementation:
----------------------------------------------------------------------
(defun LaTeX-verbatim-macro-boundaries ()
[...]
          ;; Heuristic: If an opening brace is encountered, search for
          ;; both the opening and the closing brace as an end marker.
          ;; Like that the function should work for \verb|...| as well
          ;; as for \url{...}.
          (when (string= delimiter TeX-grop)
            (setq delimiter (concat delimiter TeX-grcl)))
          (goto-char (1+ macro-end))
          (skip-chars-forward (concat "^" delimiter))
----------------------------------------------------------------------
This portion obviously indicates that this function is actually intended
for both *-delims and *-braces types of verbatim macros. Thus the line

          (verbatim-regexp (regexp-opt (LaTeX-verbatim-macros-with-delims) t)))

, a bit above the quoted portion, should actually include *-braces as
well. Then `LaTeX-verbatim-macro-boundaries' becomes able to handle both
*-delims and *-braces, and thus `LaTeX-current-verbatim-macro' does, too.

In addition, we don't have to keep the symmetric behavior of
`LaTeX-current-verbatim-macro' and `TeX-current-macro', given what I
wrote in (1). So we can modify `LaTeX-verbatim-macro-boundaries' so that
it returns nil at just before "\verb..." etc.

In this way, we can check both *-delims and *-braces by
`LaTeX-current-verbatim-macro' in `LaTeX-verbatim-p' without
`TeX-current-macro' like this:
----------------------------------------------------------------------
(defun LaTeX-verbatim-p (&optional pos)
[...]
    (or (progn
          (syntax-propertize (point))
          (nth 3 (syntax-ppss)))
        (LaTeX-current-verbatim-macro)
        (member (LaTeX-current-environment) (LaTeX-verbatim-environments)))))
----------------------------------------------------------------------

Actually, we can even omit `LaTeX-current-verbatim-macro' itself and use
`LaTeX-verbatim-macro-boundaries' instead:
----------------------------------------------------------------------
(defun LaTeX-verbatim-p (&optional pos)
[...]
    (or (progn
          (syntax-propertize (point))
          (nth 3 (syntax-ppss)))
        (LaTeX-verbatim-macro-boundaries)
        (member (LaTeX-current-environment) (LaTeX-verbatim-environments)))))
----------------------------------------------------------------------

(4) Actually, we usually need only
        (progn
          (syntax-propertize (point))
          (nth 3 (syntax-ppss)))
part in `LaTeX-verbatim-p' after all; the rest is fallback code for the
case that syntax propertize in font-latex.el wasn't used. (Unlike
`LaTeX-verbatim-macro-boundaries', `(nth 3 (syntax-ppss))' doesn't
return true on "\verb" macro itself; however it doesn't matter from what
I wrote in (2).)

Summing up the considerations above, I arrvied at the attached
tentative fix. I confirmed that it passes the regression tests.

What do you think about it?

(By the way, as written in the comment of the patch, I don't understand
the reason why the while loop in `LaTeX-verbatim-macro-boundaries' goes
back to the previous line. Is there any \verb variant, say in fancyverb
or fvextra, which allows newline in its argument?)

Regards,
Ikumi Keita
#StandWithUkraine #StopWarInUkraine

diff --git a/latex.el b/latex.el
index fb862b76..3b681b1f 100644
--- a/latex.el
+++ b/latex.el
@@ -3785,20 +3785,23 @@ values of the variable `LaTeX-verbatim-environments' as well.")
 Boundaries are returned as a cons cell where the car is the macro
 start and the cdr the macro end.
 
-Only macros which enclose their arguments with special
-non-parenthetical delimiters, like \\verb+foo+, are recognized."
+It doesn't regard the point just before \\verb... etc. belongs to
+the verbatim macro.  This isn't symmetric with
+`TeX-find-macro-boundaries'."
   (save-excursion
     (let ((orig (point))
-          (verbatim-regexp (regexp-opt (LaTeX-verbatim-macros-with-delims) t)))
-      ;; Search backwards for the macro start, unless we are facing one
-      (unless (looking-at (concat (regexp-quote TeX-esc) verbatim-regexp))
-        (catch 'found
-          (while (progn
-                   (skip-chars-backward (concat "^\n" (regexp-quote TeX-esc))
-                                        (line-beginning-position))
-                   (when (looking-at verbatim-regexp) (throw 'found nil))
-                   (or (bobp) (forward-char -1))
-                   (/= (point) (line-beginning-position))))))
+          (verbatim-regexp (regexp-opt
+                            (append (LaTeX-verbatim-macros-with-delims)
+                                    (LaTeX-verbatim-macros-with-braces))
+                            t)))
+      (catch 'found
+        (while (progn
+                 (skip-chars-backward (concat "^" (regexp-quote TeX-esc))
+                                      (line-beginning-position))
+                 (when (looking-at verbatim-regexp) (throw 'found nil))
+                 ;; XXX: Why do we go back to the preceding lines?
+                 (or (bobp) (forward-char -1))
+                 (not (bolp)))))
       ;; Search forward for the macro end, unless we failed to find a start
       (unless (bolp)
         (let* ((beg (1- (point)))
@@ -3820,27 +3823,27 @@ non-parenthetical delimiters, like \\verb+foo+, are recognized."
           (when (<= orig (point))
             (cons beg (1+ (point)))))))))
 
-(defun LaTeX-current-verbatim-macro ()
-  "Return name of verbatim macro containing point, nil if none is present."
-  (let ((macro-boundaries (LaTeX-verbatim-macro-boundaries)))
-    (when macro-boundaries
-      (save-excursion
-        (goto-char (car macro-boundaries))
-        (forward-char (length TeX-esc))
-        (buffer-substring-no-properties
-         (point) (progn (skip-chars-forward "@A-Za-z*") (point)))))))
+;; (defun LaTeX-current-verbatim-macro ()
+;;   "Return name of verbatim macro containing point, nil if none is present."
+;;   (let ((macro-boundaries (LaTeX-verbatim-macro-boundaries)))
+;;     (when macro-boundaries
+;;       (save-excursion
+;;         (goto-char (car macro-boundaries))
+;;         (forward-char (length TeX-esc))
+;;         (buffer-substring-no-properties
+;;          (point) (progn (skip-chars-forward "@A-Za-z*") (point)))))))
 
 (defun LaTeX-verbatim-p (&optional pos)
   "Return non-nil if position POS is in a verbatim-like construct."
   (when pos (goto-char pos))
   (save-match-data
-    (or (progn
+    (if (eq TeX-install-font-lock 'font-latex-setup)
+        (progn
           (syntax-propertize (point))
           (nth 3 (syntax-ppss)))
-        (member (LaTeX-current-verbatim-macro)
-                (LaTeX-verbatim-macros-with-delims))
-        (member (TeX-current-macro) (LaTeX-verbatim-macros-with-braces))
-        (member (LaTeX-current-environment) (LaTeX-verbatim-environments)))))
+      (or
+       (LaTeX-verbatim-macro-boundaries)
+       (member (LaTeX-current-environment) (LaTeX-verbatim-environments))))))

 ;;; Formatting