PATCH: keep region active when widget fails

PATCH: keep region active when widget fails

[Oliver Kiddle]

Does anyone know why we set region_active to false in handlefeep()? I
would guess that there are perhaps cases where we want this but things
like cursor left when you're already in the first position shouldn't
unset region_active. Any objections to the following patch? We can
always add the assignment to individual widgets if we find cases where
it is desirable.

Oliver

diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index e95a34b..03a2bdc 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -1365,7 +1365,6 @@ int
 handlefeep(UNUSED(char **args))
 {
     zbeep();
-    region_active = 0;
     return 0;
 }
 

Re: PATCH: keep region active when widget fails

[Bart Schaefer]

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

On Nov 6, 2014 3:55 PM, "Oliver Kiddle" <okiddle@yahoo.co.uk> wrote:
>
> Does anyone know why we set region_active to false in handlefeep()?

It's probably so that an explicit call to "zle beep" from a user-defined
widget will end the region as a side-effect ... PWS should be able to
confirm because it has been that way since the initial zle highlighting
patch.

Zle could probably use a way to signal a non-fatal error, send-break is a
bit excessive.

PATCH: Add an option to disable deactivating region hilighting whenever you edit something

[Mikael Magnusson]

For allowing zle beep to end the region, you could just use REGION_ACTIVE=0 instead.

While we're on the topic of deactivating the region, I've had this patch lying around forever in my local tree, but never cared enough to send it. It just keeps the region on until the user disables it again, I use this very complicated widget:
  _toggle_region_active () {
    (( REGION_ACTIVE = !REGION_ACTIVE ))
  }

My use case for the above is doing a set-mark-command, typing a long thing, and then invoking quote-region, while maintaining visual feedback of the region. (Sometimes I forgot to do the set-mark-command and ended up quoting the whole line).

---
 Src/Zle/zle_utils.c | 6 ++++--
 Src/options.c       | 1 +
 Src/zsh.h           | 1 +
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index ba7642b..a264a4b 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -803,7 +803,8 @@ spaceinline(int ct)
 	    }
 	}
     }
-    region_active = 0;
+    if (isset(EDITDEACTIVATESREGION))
+	region_active = 0;
 }
 
 /*
@@ -883,7 +884,8 @@ shiftchars(int to, int cnt)
 	}
 	zleline[zlell = to] = ZWC('\0');
     }
-    region_active = 0;
+    if (isset(EDITDEACTIVATESREGION))
+	region_active = 0;
 }
 
 /*
diff --git a/Src/options.c b/Src/options.c
index 45c1d11..444e9ef 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -122,6 +122,7 @@ static struct optname optns[] = {
 {{NULL, "cshnullcmd",	      OPT_EMULATE|OPT_CSH},	 CSHNULLCMD},
 {{NULL, "cshnullglob",	      OPT_EMULATE|OPT_CSH},	 CSHNULLGLOB},
 {{NULL, "debugbeforecmd",     OPT_ALL},			 DEBUGBEFORECMD},
+{{NULL, "editdeactivatesregion", OPT_ALL},               EDITDEACTIVATESREGION},
 {{NULL, "emacs",	      0},			 EMACSMODE},
 {{NULL, "equals",	      OPT_EMULATE|OPT_ZSH},	 EQUALS},
 {{NULL, "errexit",	      OPT_EMULATE},		 ERREXIT},
diff --git a/Src/zsh.h b/Src/zsh.h
index 642e290..583edd9 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2074,6 +2074,7 @@ enum {
     CSHNULLCMD,
     CSHNULLGLOB,
     DEBUGBEFORECMD,
+    EDITDEACTIVATESREGION,
     EMACSMODE,
     EQUALS,
     ERREXIT,
-- 
2.2.0-rc0

Re: PATCH: keep region active when widget fails

[Peter Stephenson]

On Fri, 07 Nov 2014 00:48:23 +0100
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Does anyone know why we set region_active to false in handlefeep()? I
> would guess that there are perhaps cases where we want this but things
> like cursor left when you're already in the first position shouldn't
> unset region_active. Any objections to the following patch? We can
> always add the assignment to individual widgets if we find cases where
> it is desirable.

That sounds OK --- I can't see any big problem.

pws

Re: PATCH: Add an option to disable deactivating region hilighting whenever you edit something

[Oliver Kiddle]

Mikael Magnusson wrote:
> 
> While we're on the topic of deactivating the region, I've had this
> patch lying around forever in my local tree, but never cared enough to
> send it. It just keeps the region on until the user disables it again, I
> use this very complicated widget:

I'm concerned that, as is often the case with options, it is a somewhat
blunt instrument. It isn't just edits that this affects. Most vi
commands too: I'd want the option set for vi command mode but not for
vi insert mode. That said, the use case you show is quite nice. The
region should be more flexible (and I have some stuff coming in this
area which I probably now need to start unloading).

Currently, the region is fixed to an emacs region model so
'region_active = 0' typically appears in shared code. We need control to
move up to individual widgets somewhat more.

I can think of a few ways this might be more versatile, setting a
flag on the zle widget so you might mark self-insert as being region
preserving might be one option. I've pondered adding this for completion
suffix removal too. We also have some prefix widgets such as for numeric
arguments which affect the behaviour of the following widget.

If we do go with the option, I'll have a think about the name.

Oliver

Re: PATCH: Add an option to disable deactivating region hilighting whenever you edit something

[Mikael Magnusson]

On Fri, Nov 7, 2014 at 11:15 AM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Mikael Magnusson wrote:
>>
>> While we're on the topic of deactivating the region, I've had this
>> patch lying around forever in my local tree, but never cared enough to
>> send it. It just keeps the region on until the user disables it again, I
>> use this very complicated widget:
>
> I'm concerned that, as is often the case with options, it is a somewhat
> blunt instrument. It isn't just edits that this affects. Most vi
> commands too: I'd want the option set for vi command mode but not for
> vi insert mode. That said, the use case you show is quite nice. The
> region should be more flexible (and I have some stuff coming in this
> area which I probably now need to start unloading).

Yeah, I've never used vi mode and I honestly didn't look into all the
cases this code is called from at all, it just fixed my case and
didn't do anything I didn't want. (This may also have been a reason
for not sending it, git send-mail apparently stripped the fact that
the patch is from 2011, so I forgot exactly what I checked or not).

> Currently, the region is fixed to an emacs region model so
> 'region_active = 0' typically appears in shared code. We need control to
> move up to individual widgets somewhat more.
>
> I can think of a few ways this might be more versatile, setting a
> flag on the zle widget so you might mark self-insert as being region
> preserving might be one option. I've pondered adding this for completion
> suffix removal too. We also have some prefix widgets such as for numeric
> arguments which affect the behaviour of the following widget.
>
> If we do go with the option, I'll have a think about the name.

I'll also admit I'm not that good at naming options :). I would be all
for more versatility as well. If we want to be super consistent and
stuff, we have the auto-suffix-remove and auto-suffix-retain widgets
already. Would that model work for this also? I suppose it would be a
bit inconvenient making a wrapper function for every single widget you
want to define this for, and/or change ones you already have wrappers
for, though. Maybe?

You can get away with just defining one wrapper function for each and
naming the widgets with a suffix that you strip in the function, and
then calling zle .$widgetstripped so it's not that bad maybe.

-- 
Mikael Magnusson