bug#29756: 25.2; Footnote-mode: recursive footnotes [PATCH included]

bug#29756: 25.2; Footnote-mode: recursive footnotes [PATCH included]

[Boruch Baum]

The current behavior of `Footnote-mode' allows recursive footnotes, in
the sense that one can create a footnote inside another footnote. The
attached suggestion for a patch merely checks that (point) is prior to
the point of the first footnote before allowing another footnote to be
created.

REPRODUCE THE BUG BEHAVIOR
==========================
1] Enter random text.
2] M-x footnote-mode
3] C-c ! a
4] Observe that emacs has generated a footnote and that point is there.
Optionally, now enter random footnote text.
5] C-c ! a
6] Observe that emacs has create a new footnote, within the current
footnote!


PROPOSED SOLUTION
=================
This snippet is presented as an advice because that's how I'm dealing
with it on my personal install, but of course the project would just
perform the check within `Footnote-add-footnote'.

(defadvice Footnote-add-footnote (around abort-when-in-fn-area activate)
  (interactive)
  (if (or
        (not footnote-text-marker-alist)
        (< (point) (cdr (first footnote-text-marker-alist))))
    ad-do-it
   (message "Add footnotes only while in text body")))


VERSION INFORMATION
===================
In GNU Emacs 25.2.2 (x86_64-pc-linux-gnu, GTK+ Version 3.22.20)
 of 2017-09-11, modified by Debian built on trouble
System Description:	Devuan GNU/Linux 1.0 (jessie)

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --build x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-x=yes --with-x-toolkit=gtk3
 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
 -fdebug-prefix-map=/build/emacs25-XrMyQe/emacs25-25.2+1=.
 -fstack-protector-strong -Wformat -Werror=format-security -Wall'
 'CPPFLAGS=-Wdate-time -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Messages

Minor modes in effect:
  global-company-mode: t
  company-mode: t
  ivy-mode: t
  sr-popviewer-mode: t
  recentf-mode: t
  shell-dirtrack-mode: t
  winner-mode: t
  show-paren-mode: t
  savehist-mode: t
  desktop-save-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  delete-selection-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#29756: 25.2; Footnote-mode: recursive footnotes [UPDATED PATCH]

[Boruch Baum]

The updated code below improves the prior patch by forbidding a user
from creating a footnote WITHIN the text of
the`footnote-section-tag-text', and covering a case of the user manuall
deleting that same footnote-section-tag-text after creating a footnote.


(defun Footnote--abort-when-in-fn-area (orig-function &optional arg)
  "Do not allow 'recursive footnotes'."
  (interactive "P")
  (let ((p (point))
        (q (if (not footnote-text-marker-alist) (point-max)
            (if (string-equal footnote-section-tag "")
              (cdr (first footnote-text-marker-alist))
             (goto-char (cdr (first footnote-text-marker-alist)))
             (if (re-search-backward
                   (concat "^" footnote-section-tag-regexp) nil t)
               (match-beginning 0)
              ; This `else' should never happen, and indicates an error, ie. footnotes
              ; already exist and a footnote-section-tag is defined, but the section tag
              ; hasn't been found. We choose to assume that the user deleted it
              ; intentionally and wants us to behave in this buffer as if the section tag
              ; was set "", so we do that, now.
              (setq footnote-section-tag "")
              ; To illustrate the difference in behavior, create a few footnotes, delete
              ; the section tag, and create another footnote. Then undo, comment the
              ; above line (that sets the tag to ""), re-evaluate this function, and repeat.
              (cdr (first footnote-text-marker-alist))
            )))))
   (goto-char p) ; undo `re-search-backward' side-effect
   (if (< p q)
      (apply orig-function arg)
    (message "Add footnotes only while in text body"))))
(advice-add 'Footnote-add-footnote :around #'Footnote--abort-when-in-fn-area)

-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#29756: 25.2; Footnote-mode: recursive footnotes [UPDATED PATCH]

[Eli Zaretskii]

> Date: Sun, 17 Dec 2017 16:21:31 -0500
> From: Boruch Baum <boruch_baum@gmx.com>
> 
> The updated code below improves the prior patch by forbidding a user
> from creating a footnote WITHIN the text of
> the`footnote-section-tag-text', and covering a case of the user manuall
> deleting that same footnote-section-tag-text after creating a footnote.

Could you please rewrite the patch without using advice?  We generally
avoid using that in Emacs's own sources.

Thanks.

bug#29756: 25.2; Footnote-mode: recursive footnotes [UPDATED PATCH]

[Boruch Baum]

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

On 2017-12-22 16:19, Eli Zaretskii wrote:
> Could you please rewrite the patch without using advice?  We generally
> avoid using that in Emacs's own sources.

Done, but in an organizationally messy manner. In parallel with this bug
report, I had proposed a separate feature on the dev mailing list, and
that snippet got swallowed into code Stefan Monnier is reviewing.

-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[-- Attachment #2: footnote.el.patch --]
[-- Type: text/x-diff, Size: 2620 bytes --]

--- /home/optimum/Downloads/footnote.el--downloaded_from_gnu.orig	2017-12-24 18:12:15.000000000 -0500
+++ /home/optimum/Downloads/footnote.el--downloaded_from_gnu	2017-12-24 18:35:34.199506628 -0500
@@ -747,6 +747,40 @@ footnote area, returns `point-max'."
 	(setq i (1+ i)))
       rc)))
 
+(defun Footnote--get-area-point-min (&optional arg)
+  "Return POINT of the beginning of the footnote area, being the
+start of the first footnote. If there is no footnote area,
+returns POINT-MAX. With optional arg, consider the
+footnote-section-tag to be part of the footnote area."
+  (if (not footnote-text-marker-alist)
+    (point-max)
+   (if (not arg)
+     (cdr (first footnote-text-marker-alist))
+    (when arg
+      (if (string-equal footnote-section-tag "")
+        (cdr (first footnote-text-marker-alist))
+       (let ((p (point)) min)
+        (goto-char (cdr (first footnote-text-marker-alist)))
+        (if (re-search-backward (concat "^" footnote-section-tag-regexp) nil t)
+          (setq min (match-beginning 0))
+         ; This `else' should never happen, and indicates an error, ie. footnotes
+         ; already exist and a footnote-section-tag is defined, but the section tag
+         ; hasn't been found. We choose to assume that the user deleted it
+         ; intentionally and wants us to behave in this buffer as if the section tag
+         ; was set "", so we do that, now.
+         (setq footnote-section-tag "")
+         ; HOWEVER: The rest of footnote mode does not currently honor or account
+         ; for this.
+         ;
+         ; To illustrate the difference in behavior, create a few footnotes, delete
+         ; the section tag, and create another footnote. Then undo, comment the
+         ; above line (that sets the tag to ""), re-evaluate this function, and repeat.
+         ;
+         ; TODO: integrate sanity checks at reasonable operational points.
+         (setq min (cdr (first footnote-text-marker-alist))))
+        (goto-char p)
+        min))))))
+
 (defun Footnote-add-footnote (&optional arg)
   "Add a numbered footnote.
 The number the footnote receives is dependent upon the relative location
@@ -755,6 +789,8 @@ If the variable `footnote-narrow-to-foot
 the buffer is narrowed to the footnote body.  The restriction is removed
 by using `Footnote-back-to-message'."
   (interactive "*P")
+  (if (> (point) (Footnote--get-area-point-min "including section tag"))
+    (error "Add footnotes only while in text body."))
   (let ((num
          (if footnote-text-marker-alist
              (if (< (point) (cl-cadar (last footnote-pointer-marker-alist)))

bug#29756: 25.2; Footnote-mode: recursive footnotes [UPDATED PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> On 2017-12-22 16:19, Eli Zaretskii wrote:
>> Could you please rewrite the patch without using advice?  We generally
>> avoid using that in Emacs's own sources.
>
> Done, but in an organizationally messy manner. In parallel with this bug
> report, I had proposed a separate feature on the dev mailing list, and
> that snippet got swallowed into code Stefan Monnier is reviewing.

Is this patch still applicable or did it get solved in conjunction with
that separate feature?

Some minor notes on the patch:

+(defun Footnote--get-area-point-min (&optional arg)
+  "Return POINT of the beginning of the footnote area, being the
+start of the first footnote. If there is no footnote area,

The first line of a comment should be a complete sentence.

+       (let ((p (point)) min)
+        (goto-char (cdr (first footnote-text-marker-alist)))

The indentation seems off?  It's often useful to run the function
through `M-C-q' to get everything to be indented properly.

+         ; This `else' should never happen, and indicates an error, ie. footnotes
+         ; already exist and a footnote-section-tag is defined, but the section tag

There should be two semicolons for in-code comments on separate lines.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#29756: 25.2; Footnote-mode: recursive footnotes [UPDATED PATCH]

[Boruch Baum]

On 2019-06-24 17:32, Lars Ingebrigtsen wrote:
> Is this patch still applicable or did it get solved in conjunction with
> that separate feature?

It's a few years old, and I only have the vaguest memory of it.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#29756: 25.2; Footnote-mode: recursive footnotes [UPDATED PATCH]

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> On 2019-06-24 17:32, Lars Ingebrigtsen wrote:
>> Is this patch still applicable or did it get solved in conjunction with
>> that separate feature?
>
> It's a few years old, and I only have the vaguest memory of it.

It's in

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29756

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#29756: 25.2; Footnote-mode: recursive footnotes [UPDATED PATCH]

[Noam Postavsky]

forcemerge 27777 29756
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Boruch Baum <boruch_baum@gmx.com> writes:
>
>> On 2019-06-24 17:32, Lars Ingebrigtsen wrote:
>>> Is this patch still applicable or did it get solved in conjunction with
>>> that separate feature?
>>
>> It's a few years old, and I only have the vaguest memory of it.
>
> It's in
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29756

I think this is the same as Bug#27777, which was recently wontfix'd; I'm
merging them (I agree with the wontfix reason: that recursive footnotes
should be allowed).