[PATCH] Enable linewise edit-command when in visual-line

[PATCH] Enable linewise edit-command when in visual-line

[Christoffer Lundell]

Change behavior of edit-command in visual-line mode to edit command linewise
instead of only allowing editing of the text between MARK and CURSOR.

The rationale being that in visual-line mode the entire line(s) appears visually
selected, and so I expect edit-command to operate on the entire line(s).

With this patch everything visually selected will be brought into the editor,
while preserving cursor position where applicable.

I am not entirely sure I handle this correctly, though it appears to work fine
when tested locally with both emacs and vim.

---
 Functions/Zle/edit-command-line | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
index 5f7ea321f..52246b88f 100644
--- a/Functions/Zle/edit-command-line
+++ b/Functions/Zle/edit-command-line
@@ -11,7 +11,7 @@ local left right prebuffer buffer=$BUFFER lbuffer=$LBUFFER
 local TMPSUFFIX=.zsh
 # set up parameters depending on which context we are called from,
 # see below comment for more details
-if (( REGION_ACTIVE )); then
+if (( REGION_ACTIVE == 1 )); then
   if (( CURSOR < MARK )); then
     left=$CURSOR right=$MARK
     lbuffer=
@@ -21,6 +21,23 @@ if (( REGION_ACTIVE )); then
   fi
   (( left++ ))
   buffer=$BUFFER[left,right]
+elif (( REGION_ACTIVE == 2 )); then
+  local mark_left mark_right offset_right
+  if (( CURSOR < MARK )); then
+    mark_left=$CURSOR mark_right=$MARK
+  else
+    mark_left=$MARK mark_right=$CURSOR
+  fi
+  left=${(SB)${BUFFER[1,mark_left]}%$'\n'}
+  (( left != 1 )) && (( left++ ))
+  offset_right=${(SB)${BUFFER[mark_right+1,-1]}#$'\n'}
+  if (( offset_right == 1 )); then
+    right=$#BUFFER
+  else
+    (( right = mark_right + offset_right - 1 ))
+  fi
+  lbuffer=${lbuffer[left,CURSOR]}
+  buffer=$BUFFER[left,right]
 elif (( ! ZLE_RECURSIVE )); then
   prebuffer=$PREBUFFER
 fi
--
2.41.0

Re: [PATCH] Enable linewise edit-command when in visual-line

[Bart Schaefer]

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

On Mon, Aug 21, 2023 at 7:14 AM Christoffer Lundell
<christofferlundell@protonmail.com> wrote:
>
> Change behavior of edit-command in visual-line mode to edit command linewise
> instead of only allowing editing of the text between MARK and CURSOR.
>
> The rationale being that in visual-line mode the entire line(s) appears visually
> selected, and so I expect edit-command to operate on the entire line(s).

This seems quite reasonable.

> With this patch everything visually selected will be brought into the editor,
> while preserving cursor position where applicable.

This looks OK, but I think it can be made a bit clearer by using the
(i)/(I) subscript indexing flags rather than ${(SB)...}.  This might
just be my own bias, but those flags already return the "correct"
values when the search fails, so the extra marks and offsets and tests
are not needed.

I'm no longer a regular vi user (and never used line-wise mode years
ago when I was) so I do have a couple of questions about your index
computations ...

> +  left=${(SB)${BUFFER[1,mark_left]}%$'\n'}
> +  (( left != 1 )) && (( left++ ))

This I follow.  If the (SB) is replaced with ...

  local nl=$'\n'
  left=${${BUFFER[1,mark_left]}[(I)$nl]}

... then $left can unconditionally be incremented because the result
will be 0 rather than 1 if there is no newline in the range.  Unless
of course  [[ $BUFFER[1] == $nl ]] in which case your version includes
that newline in the selection and mine doesn't.  Which one is correct?

The $nl is introduced because subscript brackets are parsed like
double quotes, which means you can't use $'\n' inside them.

> +  offset_right=${(SB)${BUFFER[mark_right+1,-1]}#$'\n'}

This I'm not sure of -- why mark_right+1 ?  In this direction that
seems to mean that if the mark is exactly on the newline, you're
extending to the following newline.  Is that how it's meant to work?

> +  if (( offset_right == 1 )); then
> +    right=$#BUFFER
> +  else
> +    (( right = mark_right + offset_right - 1 ))
> +  fi

I would do ...

  right=${${BUFFER[mark_right,-1]}[(i)$nl]}
  (( right += mark_right - 1 ))

... which returns the length of the range plus one if there is no
newline, so $right may be unconditionally decremented.  Again, though,
I would expect that if mark_right is on the newline, we stop there,
not skip it and find the next.

With my version the remaining tweak needed is to decrement $right a
second time when [[ $BUFFER[right] == $nl ]] because use of "$(<$1)"
trims a trailing newline when reading the back the editor file.

Unwinding a bit to remove the additional locals, the result is the attached.

[-- Attachment #2: edcomln.txt --]
[-- Type: text/plain, Size: 1252 bytes --]

diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
index 5f7ea321f..c9b140378 100644
--- a/Functions/Zle/edit-command-line
+++ b/Functions/Zle/edit-command-line
@@ -11,7 +11,7 @@ local left right prebuffer buffer=$BUFFER lbuffer=$LBUFFER
 local TMPSUFFIX=.zsh
 # set up parameters depending on which context we are called from,
 # see below comment for more details
-if (( REGION_ACTIVE )); then
+if (( REGION_ACTIVE == 1 )); then
   if (( CURSOR < MARK )); then
     left=$CURSOR right=$MARK
     lbuffer=
@@ -19,7 +19,23 @@ if (( REGION_ACTIVE )); then
     left=$MARK right=$CURSOR
     lbuffer[right-left,-1]=
   fi
-  (( left++ ))
+  buffer=$BUFFER[++left,right]
+elif (( REGION_ACTIVE == 2 )); then
+  local nl=$'\n'
+  if (( CURSOR < MARK )); then
+    left=${${BUFFER[1,CURSOR]}[(I)$nl]}
+    right=${${BUFFER[MARK,-1]}[(i)$nl]}
+    (( right += MARK ))
+  else
+    left=${${BUFFER[1,MARK]}[(I)$nl]}
+    right=${${BUFFER[CURSOR,-1]}[(i)$nl]}
+    (( right += CURSOR ))
+  fi
+  lbuffer=$BUFFER[1,++left]
+  if [[ $BUFFER[--right] = $nl ]]; then
+    # Keep the newline because "$(<$1)" below trims it
+    (( --right ))
+  fi
   buffer=$BUFFER[left,right]
 elif (( ! ZLE_RECURSIVE )); then
   prebuffer=$PREBUFFER

Re: [PATCH] Enable linewise edit-command when in visual-line

[Christoffer Lundell]

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

> > + left=${(SB)${BUFFER[1,mark_left]}%$'\n'}
> > + (( left != 1 )) && (( left++ ))
>
> This I follow. If the (SB) is replaced with ...
>
> local nl=$'\n'
> left=${${BUFFER[1,mark_left]}[(I)$nl]}

Thank you, this is much better.

>
> > + offset_right=${(SB)${BUFFER[mark_right+1,-1]}#$'\n'}
>
>
> This I'm not sure of -- why mark_right+1 ? In this direction that
> seems to mean that if the mark is exactly on the newline, you're
> extending to the following newline. Is that how it's meant to work?

This is because CURSOR and MARK are 0-indexed, according to zshzle(1). And so a
cursor at the first position in a line will have the same index as a newline of
the previous line inside an array subscript. Unless I am missing something I
would insist on the following.

right=${${BUFFER[MARK+1,-1]}[(i)$nl]}
(( right += MARK ))

which also means we later on no longer need to immediately predecrement $right,
since the matched index will be 1 lower.


There is also the issue about supporting cursor position in emacs, since now
$lbuffer is used to split the selected lines and figure out the cursor line and
column. But in both the original code and in your patch improvement, $lbuffer
will contain text from the beginning of $BUFFER regardless of where the visual
selection actually starts. This works fine in vim since they only use byte
offsets, but I would recommend changing it to the following in _both_ cases.

lbuffer=$lbuffer[++left,-1]

This leaves me with the attached suggestion (with the caveat that multiple
empty trailing lines will be trimmed to one newline)

Thank you a bunch for taking the time to help me, I understand this is not the
most exciting feature :)
Christoffer

---
 Functions/Zle/edit-command-line | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
index 5f7ea321f..d4b405eaf 100644
--- a/Functions/Zle/edit-command-line
+++ b/Functions/Zle/edit-command-line
@@ -11,15 +11,30 @@ local left right prebuffer buffer=$BUFFER lbuffer=$LBUFFER
 local TMPSUFFIX=.zsh
 # set up parameters depending on which context we are called from,
 # see below comment for more details
-if (( REGION_ACTIVE )); then
+if (( REGION_ACTIVE == 1 )); then
   if (( CURSOR < MARK )); then
     left=$CURSOR right=$MARK
-    lbuffer=
   else
     left=$MARK right=$CURSOR
-    lbuffer[right-left,-1]=
   fi
-  (( left++ ))
+  lbuffer=$lbuffer[++left,-1]
+  buffer=$BUFFER[left,++right]
+elif (( REGION_ACTIVE == 2 )); then
+  local nl=$'\n'
+  if (( CURSOR < MARK )); then
+    left=${${BUFFER[1,CURSOR]}[(I)$nl]}
+    right=${${BUFFER[MARK+1,-1]}[(i)$nl]}
+    (( right += MARK ))
+  else
+    left=${${BUFFER[1,MARK]}[(I)$nl]}
+    right=${${BUFFER[CURSOR+1,-1]}[(i)$nl]}
+    (( right += CURSOR ))
+  fi
+  lbuffer=$lbuffer[++left,-1]
+  if [[ $BUFFER[right] = $nl ]]; then
+    # Keep the newline because "$(<$1)" below trims it
+    (( --right ))
+  fi
   buffer=$BUFFER[left,right]
 elif (( ! ZLE_RECURSIVE )); then
   prebuffer=$PREBUFFER
-- 
2.41.0

[-- Attachment #2: edcomln_v3.txt --]
[-- Type: text/plain, Size: 1260 bytes --]

diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
index 5f7ea321f..d4b405eaf 100644
--- a/Functions/Zle/edit-command-line
+++ b/Functions/Zle/edit-command-line
@@ -11,15 +11,30 @@ local left right prebuffer buffer=$BUFFER lbuffer=$LBUFFER
 local TMPSUFFIX=.zsh
 # set up parameters depending on which context we are called from,
 # see below comment for more details
-if (( REGION_ACTIVE )); then
+if (( REGION_ACTIVE == 1 )); then
   if (( CURSOR < MARK )); then
     left=$CURSOR right=$MARK
-    lbuffer=
   else
     left=$MARK right=$CURSOR
-    lbuffer[right-left,-1]=
   fi
-  (( left++ ))
+  lbuffer=$lbuffer[++left,-1]
+  buffer=$BUFFER[left,++right]
+elif (( REGION_ACTIVE == 2 )); then
+  local nl=$'\n'
+  if (( CURSOR < MARK )); then
+    left=${${BUFFER[1,CURSOR]}[(I)$nl]}
+    right=${${BUFFER[MARK+1,-1]}[(i)$nl]}
+    (( right += MARK ))
+  else
+    left=${${BUFFER[1,MARK]}[(I)$nl]}
+    right=${${BUFFER[CURSOR+1,-1]}[(i)$nl]}
+    (( right += CURSOR ))
+  fi
+  lbuffer=$lbuffer[++left,-1]
+  if [[ $BUFFER[right] = $nl ]]; then
+    # Keep the newline because "$(<$1)" below trims it
+    (( --right ))
+  fi
   buffer=$BUFFER[left,right]
 elif (( ! ZLE_RECURSIVE )); then
   prebuffer=$PREBUFFER
-- 
2.41.0

Re: [PATCH] Enable linewise edit-command when in visual-line

[Bart Schaefer]

On Wed, Aug 23, 2023 at 2:06 AM Christoffer Lundell
<christofferlundell@protonmail.com> wrote:
>
> > > + offset_right=${(SB)${BUFFER[mark_right+1,-1]}#$'\n'}
> >
> > This I'm not sure of -- why mark_right+1 ?
>
> This is because CURSOR and MARK are 0-indexed, according to zshzle(1).

They're 0-based because position 0 represents an empty line, the
non-character before $LBUFFER.  The BUFFER and LBUFFER arrays
themselves are still 1-indexed (unless you're crazed enough to be
using zle with ksharrays).

BUFFER[CURSOR] is always the character to the left of the visible
cursor position.  To make this work, CURSOR is decremented when
entering vicmd mode (unless it's already zero), but that doesn't
change MARK.

So ... I think this means that in vi-cmd-mode it is necessary to use
CURSOR+1 when MARK <= CURSOR, but not MARK+1 when CURSOR < MARK?  And
never add one when in emacs mode?

This bothers me a little because starting from CURSOR < MARK,
exchange-point-and-mark (or vi-goto-mark?) could extend the range one
full line to the right?

> which also means we later on no longer need to immediately predecrement $right,
> since the matched index will be 1 lower.

I'm not sure whether the foregoing affects that, and can't experiment right now.

> There is also the issue about supporting cursor position in emacs, since now
> $lbuffer is used to split the selected lines and figure out the cursor line and
> column. But in both the original code and in your patch improvement, $lbuffer
> will contain text from the beginning of $BUFFER regardless of where the visual
> selection actually starts. This works fine in vim since they only use byte
> offsets, but I would recommend changing it to the following in _both_ cases.
>
> lbuffer=$lbuffer[++left,-1]

That's equivalent to lbuffer= when left=$CURSOR because by definition
everything after LBUFFER[CURSOR] is empty (and lbuffer=$LBUFFER at
that point).  I believe ((right-left == left+1)) when right=$CURSOR,
so should be a no-op compared to the original ((REGION_HIGHLIGHT ==
1)) case.  Disagree?

Re: [PATCH] Enable linewise edit-command when in visual-line

[Christoffer Lundell]

Thank you for your fast response, I did a bit of testing and have a couple
uncertainties that I hope you can assist me with.

> > > > + offset_right=${(SB)${BUFFER[mark_right+1,-1]}#$'\n'}
> > > 
> > > This I'm not sure of -- why mark_right+1 ?
> > 
> > This is because CURSOR and MARK are 0-indexed, according to zshzle(1).
> 
> They're 0-based because position 0 represents an empty line, the
> non-character before $LBUFFER. The BUFFER and LBUFFER arrays
> themselves are still 1-indexed (unless you're crazed enough to be
> using zle with ksharrays).
> BUFFER[CURSOR] is always the character to the left of the visible
> cursor position. To make this work, CURSOR is decremented when
> entering vicmd mode (unless it's already zero), but that doesn't
> change MARK.
> 
> So ... I think this means that in vi-cmd-mode it is necessary to use
> CURSOR+1 when MARK <= CURSOR, but not MARK+1 when CURSOR < MARK? And
> never add one when in emacs mode?
> 

This makes me think I am missing something, because even when setting bindkey -e
in my .zshrc and entering visual-line-mode, a cursor/mark on column 1 on a 2nd
line to the right of the other cursor/mark never includes said 2nd line, despite
it being visually highlighted. That is, while using the suggested:
  right=${${BUFFER[CURSOR,-1]}[(i)$nl]} for MARK <= CURSOR
and
  right=${${BUFFER[MARK,-1]}[(i)$nl]} for CURSOR < MARK

I had to use CURSOR+1 and MARK+1 from the lines above to fix that edge case.
Which does not wrongfully include the following line when the cursor to the far
right of the first line in emacs mode, as might be expected.

> > lbuffer=$lbuffer[++left,-1]
> 
> That's equivalent to lbuffer= when left=$CURSOR because by definition
> everything after LBUFFER[CURSOR] is empty (and lbuffer=$LBUFFER at
> that point). I believe ((right-left == left+1)) when right=$CURSOR,
> so should be a no-op compared to the original ((REGION_HIGHLIGHT ==
> 1)) case. Disagree?

You are right in that lbuffer= is equivalent for left=$CURSOR, however
lbuffer[right-left,-1]= leaves the leading part of $lbuffer intact even if the
visual selection does not start at the beginning of $BUFFER. And as I
understand, $lbuffer is supposed to represent only the highlighted text before
the cursor, to later figure out cursor positioning for the opened editor.

The only thing that has been working consistently, and what fixes cursor
positioning for me, is the previous:
  lbuffer=${lbuffer[++left,-1]}
In both blocks for (( REGION_HIGHLIGHT == 1 )) and (( REGION_HIGHLIGHT == 2 ))

Please let me know if I am missing something.

Christoffer

Re: [PATCH] Enable linewise edit-command when in visual-line

[Bart Schaefer]

On Sun, Aug 27, 2023 at 4:39 AM Christoffer Lundell
<christofferlundell@protonmail.com> wrote:
>
> > So ... I think this means that in vi-cmd-mode it is necessary to use
> > CURSOR+1 when MARK <= CURSOR, but not MARK+1 when CURSOR < MARK? And
> > never add one when in emacs mode?
>
> This makes me think I am missing something, because even when setting bindkey -e
> in my .zshrc and entering visual-line-mode, a cursor/mark on column 1 on a 2nd
> line to the right of the other cursor/mark never includes said 2nd line, despite
> it being visually highlighted. That is, while using the suggested:
>   right=${${BUFFER[CURSOR,-1]}[(i)$nl]} for MARK <= CURSOR

What I suggested in the excerpt above was that for MARK <= CURSOR this should be
  right=${${BUFFER[CURSOR+1,-1]}[(i)$nl]}
when in line-wise mode.  Which is what you have in your v3 patch.

> and
>   right=${${BUFFER[MARK,-1]}[(i)$nl]} for CURSOR < MARK
>
> I had to use CURSOR+1 and MARK+1 from the lines above to fix that edge case.

OK, one thing *I* was missing was that when entering line-wise, the
full line containing MARK is also highlighted.  Given that, yes,
MARK+1 is correct here.

I'm still a little bothered about exchange-point-and-mark.  When
CURSOR > MARK, your v3 patch slurps up everything up to and including
the character under the cursor.  If I then do exchange-point-and-mark,
the region has now lost one character from the right (because that
character is no longer under the cursor).  If, before
exchange-point-and-mark, I enter vicmd mode, this happens differently,
that is, the character under the cursor in vicmd remains in the
region, but I've still lost one character when the cursor moved left
on entry to vicmd (unless it was already at start of a line).

Thus in emacs mode, either that character should never have been in
the region in the first place, or exchange-point-and-mark has a side
effect on the region.  Anyone (other than Christoffer) have feedback
on that?

> > > lbuffer=$lbuffer[++left,-1]
>
> You are right in that lbuffer= is equivalent for left=$CURSOR, however
> lbuffer[right-left,-1]= leaves the leading part of $lbuffer intact even if the
> visual selection does not start at the beginning of $BUFFER.
>
> The only thing that has been working consistently, and what fixes cursor
> positioning for me, is the previous:
>   lbuffer=${lbuffer[++left,-1]}
> In both blocks for (( REGION_HIGHLIGHT == 1 )) and (( REGION_HIGHLIGHT == 2 ))

When (( REGION_HIGHLIGHT == 1 )) there are only two possible cursor
placements:  At the beginning of the first editor line for CURSOR <
MARK, and at the end of the last line for CURSOR > MARK, because the
editor is supposed to contain only the region between CURSOR and MARK
inclusive. (The editor gets an empty file when CURSOR == MARK.)  I
think the prevailing presumption was that so long as $#lbuffer >
CURSOR - MARK, the cursor should end up at the end of the last line
... but it appears that nowadays vim will (un)helpfully add space
padding when the "go" argument is larger than the file size, so the
"optimization" of merely shortening $lbuffer won't work in that case
any more.

> And as I
> understand, $lbuffer is supposed to represent only the highlighted text before
> the cursor, to later figure out cursor positioning for the opened editor.

That means $lbuffer has to be empty when CURSOR < MARK and not
line-wise, so calculating $lbuffer[++left,-1] is correct (because
++left >= CURSOR).  I suppose the added effort is small, so the cases
may as well be unified.

Obviously most people who use this don't use it with ranges set.

I'm now satisfied that except for the exchange-point curiosity, your
v3 patch is correct.