unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Dmitry Gutov <dgutov@yandex.ru>
To: Eli Zaretskii <eliz@gnu.org>
Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
Subject: Re: locked narrowing in ELisp
Date: Thu, 18 Aug 2022 02:13:30 +0300	[thread overview]
Message-ID: <fbd24d93-935e-8b29-cc8d-708dda26c06a@yandex.ru> (raw)
In-Reply-To: <83mtc3c5pp.fsf@gnu.org>

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

On 17.08.2022 17:20, Eli Zaretskii wrote:
>> Date: Wed, 17 Aug 2022 17:03:46 +0300
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> On 17.08.2022 16:55, Eli Zaretskii wrote:
>>>> For instance, use two overlays in the current buffer with `invisible'
>>>> property rather than have the display engine refer to the new kind of
>>>> narrowing bounds.
>>>
>>> That's a time bomb waiting to go off, because invisible text is
>>> handled at a relatively high level in the display engine, and
>>> otherwise the invisible property is largely ignored in Emacs.
>>
>> User-level features should be implementable in terms of primitives
>> allowed in Lisp userland.
> 
> I don't see how this is relevant to the concern I raised.  I was
> talking about the effects on the display engine.  It doesn't only
> display, it also looks at buffer text for various purposes.

I guess I didn't understand your concern, sorry. Invisible is handled 
somewhere on the high level, OK. I did not mistake it for 'intangible'.

>>> Moreover, it will make redisplay slower.  Skipping invisible text is
>>> faster than iterating through it, but it still takes time, whereas not
>>> going beyond BEGV..ZV is instantaneous.
>>
>> Org, as one example, uses invisible text all the time. Other feature too.
> 
> And Org is indeed relatively slow when you move through a buffer which
> has large parts of it hidden by invisible properties.

Org uses different properties, a lot. Not just 'invisible'. So I'd 
rather people test the performance of this first before dismissing.

My limited testing didn't show any particular slowdown.

>>> And finally, I don't think I see the benefit of this, even if it'd
>>> work: you want to get rid of (save-restriction (widen)), but you are
>>> willing to have to replace that with tests of overlays and invisible
>>> text all over the place?
>>
>> No, I don't think the addition of "tests ... all over the place" will be
>> needed. The display engine handles the 'invisible' property already.
>>
>> A number of features/commands will indeed need to know the bounds of the
>> user-level narrowing (and we'll have a buffer-local variable for that),
>> but that's probably it.
> 
> I don't think you realize how widespread is use of low-level
> primitives and functions in user-level commands.

Commands that do want to obey narrowing, can take the soft-narrowing 
bounds and apply the narrowing internally.

> What you suggest is not a clean design, because it is based on
> inaccurate mental model of Emacs internals.  It cannot work reliably,
> to the best of my knowledge.

I'm likely missing a lot of things, since I don't usually use this 
feature interactively. All I know is, about once or twice a year, people 
come and ask to make a certain command ignore narrowing. And nobody 
every comes and asks to revert those changes.

Could someone give a few problem scenarios with this patch? Preferably 
ones that should be hard to fix. Adapting isearch took about 2 lines.

[-- Attachment #2: soft-narrow-and-widen.diff --]
[-- Type: text/x-patch, Size: 3613 bytes --]

diff --git a/lisp/bindings.el b/lisp/bindings.el
index 2e32128274..109a1a411f 100644
--- a/lisp/bindings.el
+++ b/lisp/bindings.el
@@ -968,8 +968,8 @@ left-word
 
 (defvar-keymap narrow-map
   :doc "Keymap for narrowing commands."
-  "n" #'narrow-to-region
-  "w" #'widen
+  "n" #'soft-narrow-to-region
+  "w" #'soft-widen
   "g" #'goto-line-relative)
 (define-key ctl-x-map "n" narrow-map)
 
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 31fcf01949..285c92f850 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1321,9 +1321,10 @@ isearch-mode
   ;; Pushing the initial state used to be before running isearch-mode-hook,
   ;; but a hook might set `isearch-push-state-function' used in
   ;; `isearch-push-state' to save mode-specific initial state.  (Bug#4994)
-  (isearch-push-state)
+  (with-soft-narrow
+   (isearch-push-state)
 
-  (isearch-update)
+   (isearch-update))
 
   (add-hook 'pre-command-hook 'isearch-pre-command-hook)
   (add-hook 'post-command-hook 'isearch-post-command-hook)
@@ -1358,7 +1359,7 @@ isearch-update
 
   (if (and (null unread-command-events)
 	   (null executing-kbd-macro))
-      (progn
+      (with-soft-narrow
         (if (not (input-pending-p))
           (funcall (or isearch-message-function #'isearch-message)))
         (if (and isearch-slow-terminal-mode
diff --git a/lisp/simple.el b/lisp/simple.el
index 1e6e5e11e0..78d0804ee8 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1647,6 +1647,14 @@ goto-line
       (widen))
     (goto-char pos)))
 
+(defmacro with-soft-narrow (&rest body)
+  (declare (indent 1) (debug t))
+  `(let ((bounds (soft-narrow-bounds)))
+     (save-restriction
+       (when bounds
+         (narrow-to-region (car bounds) (cdr bounds)))
+       ,@body)))
+
 (defun goto-line-relative (line &optional buffer)
   "Go to LINE, counting from line at (point-min).
 The line number is relative to the accessible portion of the narrowed
@@ -1654,7 +1662,8 @@ goto-line-relative
   (declare (interactive-only forward-line))
   (interactive (goto-line-read-args t))
   (with-suppressed-warnings ((interactive-only goto-line))
-    (goto-line line buffer t)))
+    (with-soft-narrow
+      (goto-line line buffer t))))
 
 (defun count-words-region (start end &optional arg)
   "Count the number of words in the region.
@@ -10707,6 +10716,40 @@ lax-plist-put
   "Change value in PLIST of PROP to VAL, comparing with `equal'."
   (declare (obsolete plist-put "29.1"))
   (plist-put plist prop val #'equal))
+
+(defvar soft-narrow--overlays nil)
+
+(defun soft-widen ()
+  (interactive)
+  (when soft-narrow--overlays
+    (with-soft-narrow
+      ;; If cursor is after the cdr ovl or before car ovl,
+      ;; move it inside.
+      (delete-overlay (car soft-narrow--overlays))
+      (delete-overlay (cdr soft-narrow--overlays)))
+    (setq soft-narrow--overlays nil)))
+
+(defun soft-narrow-to-region (beg end)
+  (interactive "r")
+  (soft-widen)
+  (let ((o1 (make-overlay (point-min) beg))
+        (o2 (make-overlay end (point-max) nil t t)))
+    (overlay-put o1 'invisible t)
+    (overlay-put o1 'read-only t)
+    (overlay-put o1 'modification-hooks '(soft-narrow--modif))
+    (overlay-put o2 'invisible t)
+    (overlay-put o2 'read-only t)
+    (overlay-put o2 'modification-hooks '(soft-narrow--modif))
+    (setq soft-narrow--overlays (cons o1 o2))))
+
+(defun soft-narrow--modif (&rest _)
+  (user-error "Not allowed"))
+
+(defun soft-narrow-bounds ()
+  (when soft-narrow--overlays
+    (cons (overlay-end (car soft-narrow--overlays))
+          (overlay-start (cdr soft-narrow--overlays)))))
+
 \f
 
 (provide 'simple)

  reply	other threads:[~2022-08-17 23:13 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-16 20:18 locked narrowing in ELisp Stefan Monnier
2022-08-17  0:05 ` Dmitry Gutov
2022-08-17  0:55   ` Stefan Monnier
2022-08-17  1:00     ` Dmitry Gutov
2022-08-17 13:03       ` Stefan Monnier
2022-08-17 13:40         ` Dmitry Gutov
2022-08-17 13:55           ` Eli Zaretskii
2022-08-17 14:03             ` Dmitry Gutov
2022-08-17 14:20               ` Eli Zaretskii
2022-08-17 23:13                 ` Dmitry Gutov [this message]
2022-08-18  1:58                   ` Stefan Monnier
2022-08-18 21:42                     ` Dmitry Gutov
2022-08-18  6:25                   ` Eli Zaretskii
2022-08-18 23:10                     ` Dmitry Gutov
2022-08-19  6:31                       ` Eli Zaretskii
2022-08-22  0:59                         ` Dmitry Gutov
2022-08-17 11:44   ` Eli Zaretskii
2022-08-17 11:54     ` Dmitry Gutov
2022-08-17  5:59 ` Po Lu

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.gnu.org/software/emacs/

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

  git send-email \
    --in-reply-to=fbd24d93-935e-8b29-cc8d-708dda26c06a@yandex.ru \
    --to=dgutov@yandex.ru \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /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.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).