On controlling rectangle-preview

On controlling rectangle-preview

[Mark Oteiza]

The automatic preview can be pretty jarring, especially when doing
rectangle-string in one place (some mode, language, what-have-you), then
going someplace else, doing rectangle-string and seeing the preview when
the plan is to enter something completely different.

To avoid this I'd like to prevent the preview unless text is entered
into the minibuffer by typing, M-{n,p}, or otherwise.

Not sure if there is a better way to control this other than introducing
a parameter. For the reasons above I'd prefer this to be nil by default,
but whatever:

diff --git a/lisp/rect.el b/lisp/rect.el
index 8803a47..9c8fb29 100644
--- a/lisp/rect.el
+++ b/lisp/rect.el
@@ -407,10 +407,15 @@ rectangle-preview
   :version "25.1"
   :type 'boolean)
 
+(defcustom rectangle-preview-default t
+  "If non-nil, `string-rectangle' shows the default without minibuffer input."
+  :version "25.2"
+  :type 'boolean)
+
 (defun rectangle--string-preview ()
   (when rectangle-preview
     (let ((str (minibuffer-contents)))
-      (when (equal str "")
+      (when (and rectangle-preview-default (equal str ""))
         (setq str (or (car-safe minibuffer-default)
                       (if (stringp minibuffer-default) minibuffer-default))))
       (when str (setq str (propertize str 'face 'rectangle-preview)))
-- 

Re: On controlling rectangle-preview

[Stefan Monnier]

> The automatic preview can be pretty jarring, especially when doing
> rectangle-string in one place (some mode, language, what-have-you), then
> going someplace else, doing rectangle-string and seeing the preview when
> the plan is to enter something completely different.

FWIW, I also find it unsatisfactory in some cases.  Which cases annoy me
depends on whether the text I'm about to insert is related to the last
text I inserted via that mechanism, so there's no way Emacs can tell
whether it'll be annoying or not.

> Not sure if there is a better way to control this other than introducing
> a parameter.

Agreed.  I thought about delaying the preview a bit when str is "", but
it's fiddly and not very convincing either.

> For the reasons above I'd prefer this to be nil by default,

I could live with it being nil by default (or even without any option
at all).

> -      (when (equal str "")
> +      (when (and rectangle-preview-default (equal str ""))

But I think the preview should not be inhibited when str is "" *again*
(i.e. when str is "" as a result of minibuffer modifications rather than
as a result of inaction).


        Stefan

Re: On controlling rectangle-preview

[Mark Oteiza]

On 19/07/16 at 09:32pm, Stefan Monnier wrote:
> > The automatic preview can be pretty jarring, especially when doing
> > rectangle-string in one place (some mode, language, what-have-you), then
> > going someplace else, doing rectangle-string and seeing the preview when
> > the plan is to enter something completely different.
> 
> FWIW, I also find it unsatisfactory in some cases.  Which cases annoy me
> depends on whether the text I'm about to insert is related to the last
> text I inserted via that mechanism, so there's no way Emacs can tell
> whether it'll be annoying or not.
> 
> > Not sure if there is a better way to control this other than introducing
> > a parameter.
> 
> Agreed.  I thought about delaying the preview a bit when str is "", but
> it's fiddly and not very convincing either.
> 
> > For the reasons above I'd prefer this to be nil by default,
> 
> I could live with it being nil by default (or even without any option
> at all).
> 
> > -      (when (equal str "")
> > +      (when (and rectangle-preview-default (equal str ""))
> 
> But I think the preview should not be inhibited when str is "" *again*
> (i.e. when str is "" as a result of minibuffer modifications rather than
> as a result of inaction).

To me that seems a little odd (unexpected behaviour), and with an empty
minibuffer the default preview is only an M-n away.

Since rectangle--string-preview's scope seems to just be in the
minibuffer it looks like it can be done by locally setting
rectangle-preview-default => t (or otherwise making that condition
truthy) after the first `let' in rectangle--string-preview.

Another parameter?  Or rectangle-preview could be made a ternary-or-so option.

Re: On controlling rectangle-preview

[Stefan Monnier]

> To me that seems a little odd (unexpected behaviour),

Hmm... I can see that.

> and with an empty minibuffer the default preview is only an M-n away.

Good point.  So I think your patch is OK as is, tho I'm not sure the
custom variable is worth the trouble.


        Stefan

Re: On controlling rectangle-preview

[Mark Oteiza]

On 20/07/16 at 08:37am, Stefan Monnier wrote:
> > To me that seems a little odd (unexpected behaviour),
> 
> Hmm... I can see that.
> 
> > and with an empty minibuffer the default preview is only an M-n away.
> 
> Good point.  So I think your patch is OK as is, tho I'm not sure the
> custom variable is worth the trouble.

Sounds good to me, thanks.

This ends up bring very simple--permission to push to emacs-25?

diff --git a/lisp/rect.el b/lisp/rect.el
index 8803a47..a86d155 100644
--- a/lisp/rect.el
+++ b/lisp/rect.el
@@ -410,9 +410,6 @@ rectangle-preview
 (defun rectangle--string-preview ()
   (when rectangle-preview
     (let ((str (minibuffer-contents)))
-      (when (equal str "")
-        (setq str (or (car-safe minibuffer-default)
-                      (if (stringp minibuffer-default) minibuffer-default))))
       (when str (setq str (propertize str 'face 'rectangle-preview)))
       (with-selected-window rectangle--string-preview-window
         (unless (or (null rectangle--string-preview-state)

Re: On controlling rectangle-preview

[John Wiegley]

>>>>> Mark Oteiza <mvoteiza@udel.edu> writes:

> This ends up bring very simple--permission to push to emacs-25?

If Eli agrees, it looks safe enough to me for the release.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: On controlling rectangle-preview

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org,  Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 21 Jul 2016 00:03:25 -0700
> 
> > This ends up bring very simple--permission to push to emacs-25?
> 
> If Eli agrees, it looks safe enough to me for the release.

Sorry, I'm confused by the last message in the discussion of the
patch.  This started by introducing a new defcustom that controls the
preview, but ended with a patch adding no defcustom and instead
deleting 3 lines?  What did I miss (specifically between
http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00845.html,
which approves the original patch, and
http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00911.html,
which presents a totally different one)?

And why is it needed in emacs-25?  Is there some urgent or
particularly nasty problem involved that we'd like to fix ASAP?

Thanks.

Re: On controlling rectangle-preview

[Mark Oteiza]

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

On Jul 21, 2016 10:28 AM, "Eli Zaretskii" <eliz@gnu.org> wrote:
>
> > From: John Wiegley <jwiegley@gmail.com>
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org,
Eli Zaretskii <eliz@gnu.org>
> > Date: Thu, 21 Jul 2016 00:03:25 -0700
> >
> > > This ends up bring very simple--permission to push to emacs-25?
> >
> > If Eli agrees, it looks safe enough to me for the release.
>
> Sorry, I'm confused by the last message in the discussion of the
> patch.  This started by introducing a new defcustom that controls the
> preview, but ended with a patch adding no defcustom and instead
> deleting 3 lines?  What did I miss (specifically between
> http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00845.html,
> which approves the original patch, and

The new defcustom in the original patch would only prevent the preview if
its value was nil and the minibuffer was empty.  If we agree on nil by
default instead, and if the new variable is not worth adding, then...

> http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00911.html,
> which presents a totally different one)?

... this is the patch that achieves that. Of course, I could have
misinterpreted Stefan's comments here
http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00835.html
Sorry if that is the case.

> And why is it needed in emacs-25?  Is there some urgent or
> particularly nasty problem involved that we'd like to fix ASAP?

It affects a feature that will be new in the release, as opposed to
affecting a feature that existed in previous releases.

[-- Attachment #2: Type: text/html, Size: 2331 bytes --]

Re: On controlling rectangle-preview

[Stefan Monnier]

>> http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00911.html,
>> which presents a totally different one)?
> ... this is the patch that achieves that.

Indeed.

> Of course, I could have
> misinterpreted Stefan's comments here
> http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00835.html

No, you got it right.


        Stefan

Re: On controlling rectangle-preview

[Eli Zaretskii]

> Date: Thu, 21 Jul 2016 10:54:22 -0400
> From: Mark Oteiza <mvoteiza@udel.edu>
> Cc: John Wiegley <jwiegley@gmail.com>,
> 	Stefan Monnier <monnier@iro.umontreal.ca>,
> 	Emacs-devel <emacs-devel@gnu.org>
> 
> The new defcustom in the original patch would only prevent the preview if its value was nil and the minibuffer
> was empty. If we agree on nil by default instead, and if the new variable is not worth adding, then... 
> 
> > http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00911.html,
> > which presents a totally different one)?
> 
> ... this is the patch that achieves that. Of course, I could have misinterpreted Stefan's comments here
> http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00835.html
> Sorry if that is the case. 
> 
> > And why is it needed in emacs-25? Is there some urgent or
> > particularly nasty problem involved that we'd like to fix ASAP?
> 
> It affects a feature that will be new in the release, as opposed to affecting a feature that existed in previous
> releases.

Thanks, I'm okay with pushing it to emacs-25.

Re: On controlling rectangle-preview

[Mark Oteiza]

On 21/07/16 at 07:06pm, Eli Zaretskii wrote:
> > > And why is it needed in emacs-25? Is there some urgent or
> > > particularly nasty problem involved that we'd like to fix ASAP?
> > 
> > It affects a feature that will be new in the release, as opposed to
> > affecting a feature that existed in previous releases.
> 
> Thanks, I'm okay with pushing it to emacs-25.

Done, thanks.