unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
@ 2018-08-22 15:47 Daniel Tam
  2018-08-23  2:42 ` Mark H Weaver
  2022-07-11 12:20 ` bug#32501: Bug #32501 - " Sean
  0 siblings, 2 replies; 9+ messages in thread
From: Daniel Tam @ 2018-08-22 15:47 UTC (permalink / raw)
  To: 32501

Hi,


I've activated readline support for the Guile repl, but I've found that
if my inputrc enables vi-mode, then the bouncing parentheses feature
doesn't work. Disabling vi-mode does the trick.


Cheers,

Dan







^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-22 15:47 bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled Daniel Tam
@ 2018-08-23  2:42 ` Mark H Weaver
  2018-08-23  4:03   ` Mark H Weaver
  2018-08-24  5:36   ` Mark H Weaver
  2022-07-11 12:20 ` bug#32501: Bug #32501 - " Sean
  1 sibling, 2 replies; 9+ messages in thread
From: Mark H Weaver @ 2018-08-23  2:42 UTC (permalink / raw)
  To: Daniel Tam; +Cc: 32501

Hi Daniel,

Daniel Tam <danooo.tam@gmail.com> writes:

> I've activated readline support for the Guile repl, but I've found that
> if my inputrc enables vi-mode, then the bouncing parentheses feature
> doesn't work. Disabling vi-mode does the trick.

Indeed.  For some reason that I cannot determine, the bouncing
parentheses feature is specifically disabled when the vi keymap is in
use.  The relevant function is 'init_bouncing_parens' in
guile-readline/readline.c:

  static void
  init_bouncing_parens ()
  {
    if (strncmp (rl_get_keymap_name (rl_get_keymap ()), "vi", 2))
      {
        rl_bind_key (')', match_paren);
        rl_bind_key (']', match_paren);
        rl_bind_key ('}', match_paren);
      }
  }

This is ancient code, predating version control, present in the original
import into CVS in 1999.

I looked at the source code of readline-7.0, and IIUC none of those keys
have mappings in the default vi keymap.

The right fix might be to simply remove the 'if' check above.  Would you
like to try it and report back?

      Thanks,
        Mark





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-23  2:42 ` Mark H Weaver
@ 2018-08-23  4:03   ` Mark H Weaver
  2018-08-23 16:13     ` John Cowan
  2018-08-24  5:36   ` Mark H Weaver
  1 sibling, 1 reply; 9+ messages in thread
From: Mark H Weaver @ 2018-08-23  4:03 UTC (permalink / raw)
  To: Daniel Tam; +Cc: 32501

Hi again,

> Daniel Tam <danooo.tam@gmail.com> writes:
>
>> I've activated readline support for the Guile repl, but I've found that
>> if my inputrc enables vi-mode, then the bouncing parentheses feature
>> doesn't work. Disabling vi-mode does the trick.
>
> Indeed.  For some reason that I cannot determine, the bouncing
> parentheses feature is specifically disabled when the vi keymap is in
> use.

I think I now see the reason for it.  I noticed that readline's default
vi keymap includes a binding for '%', which jumps to the paren matching
the one under the cursor.  That reminded me, from many years ago when I
used vi more often, that this is the way that old vi traditionally
allows matching parens to be found.

So, I guess the decision long ago to disable bouncing parens when in vi
mode was to match the way that emacs and vi behaved at that time.

However, I just tried modern vim, and I see that it now highlights
matching parens by default.  So, we should probably remove the 'if' to
match this newer behavior.

       Mark





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-23  4:03   ` Mark H Weaver
@ 2018-08-23 16:13     ` John Cowan
  0 siblings, 0 replies; 9+ messages in thread
From: John Cowan @ 2018-08-23 16:13 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: danooo.tam, 32501

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

Indeed, I pretty much only go into vi-mode in vi itself to use %; the rest
of the time, I remain an "ex" troglodyte whose only vi-commands are h, j,
k, l (and arrow analogues), x, %, and most importantly gQ.  Yes, even when
writing Lisp.

On Thu, Aug 23, 2018 at 12:05 AM Mark H Weaver <mhw@netris.org> wrote:

> Hi again,
>
> > Daniel Tam <danooo.tam@gmail.com> writes:
> >
> >> I've activated readline support for the Guile repl, but I've found that
> >> if my inputrc enables vi-mode, then the bouncing parentheses feature
> >> doesn't work. Disabling vi-mode does the trick.
> >
> > Indeed.  For some reason that I cannot determine, the bouncing
> > parentheses feature is specifically disabled when the vi keymap is in
> > use.
>
> I think I now see the reason for it.  I noticed that readline's default
> vi keymap includes a binding for '%', which jumps to the paren matching
> the one under the cursor.  That reminded me, from many years ago when I
> used vi more often, that this is the way that old vi traditionally
> allows matching parens to be found.
>
> So, I guess the decision long ago to disable bouncing parens when in vi
> mode was to match the way that emacs and vi behaved at that time.
>
> However, I just tried modern vim, and I see that it now highlights
> matching parens by default.  So, we should probably remove the 'if' to
> match this newer behavior.
>
>        Mark
>
>
>
>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-23  2:42 ` Mark H Weaver
  2018-08-23  4:03   ` Mark H Weaver
@ 2018-08-24  5:36   ` Mark H Weaver
  2018-08-24  6:09     ` Mark H Weaver
  1 sibling, 1 reply; 9+ messages in thread
From: Mark H Weaver @ 2018-08-24  5:36 UTC (permalink / raw)
  To: Daniel Tam; +Cc: 32501

Hello again,

I wrote:

> The relevant function is 'init_bouncing_parens' in
> guile-readline/readline.c:
>
>   static void
>   init_bouncing_parens ()
>   {
>     if (strncmp (rl_get_keymap_name (rl_get_keymap ()), "vi", 2))
>       {
>         rl_bind_key (')', match_paren);
>         rl_bind_key (']', match_paren);
>         rl_bind_key ('}', match_paren);
>       }
>   }

[...]

> The right fix might be to simply remove the 'if' check above.

Actually, it probably won't be that simple.  Whereas in emacs mode, we
can probably rely on these bindings being added to the correct keymap,
namely 'emacs_standard_keymap', when in vi mode it's likely that these
bindings would end up in the wrong keymap, namely 'vi_movement_keymap'.

When in vi mode, these mappings should be added to
'vi_insertion_keymap', and that probably involves using
'rl_bind_key_in_map' instead of 'rl_bind_key'.

       Mark





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-24  5:36   ` Mark H Weaver
@ 2018-08-24  6:09     ` Mark H Weaver
  2018-08-24 12:42       ` Daniel Tam
  0 siblings, 1 reply; 9+ messages in thread
From: Mark H Weaver @ 2018-08-24  6:09 UTC (permalink / raw)
  To: Daniel Tam; +Cc: 32501

I wrote:

>> The right fix might be to simply remove the 'if' check above.
>
> Actually, it probably won't be that simple.  Whereas in emacs mode, we
> can probably rely on these bindings being added to the correct keymap,
> namely 'emacs_standard_keymap', when in vi mode it's likely that these
> bindings would end up in the wrong keymap, namely 'vi_movement_keymap'.
>
> When in vi mode, these mappings should be added to
> 'vi_insertion_keymap', and that probably involves using
> 'rl_bind_key_in_map' instead of 'rl_bind_key'.

And I guess 'rl_get_keymap_by_name' is the function we should use to get
the keymap.  From keymaps.h in readline:

  /* Return the keymap corresponding to a given name.  Names look like
     `emacs' or `emacs-meta' or `vi-insert'.  */
  extern Keymap rl_get_keymap_by_name PARAMS((const char *));

      Mark





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-24  6:09     ` Mark H Weaver
@ 2018-08-24 12:42       ` Daniel Tam
  2018-08-24 22:11         ` Mark H Weaver
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Tam @ 2018-08-24 12:42 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: 32501

Hey Mark,


On 08/24/2018 04:09 PM, Mark H Weaver wrote:
> I wrote:
>
>>> The right fix might be to simply remove the 'if' check above.
>> Actually, it probably won't be that simple.  Whereas in emacs mode, we
>> can probably rely on these bindings being added to the correct keymap,
>> namely 'emacs_standard_keymap', when in vi mode it's likely that these
>> bindings would end up in the wrong keymap, namely 'vi_movement_keymap'.
>>
>> When in vi mode, these mappings should be added to
>> 'vi_insertion_keymap', and that probably involves using
>> 'rl_bind_key_in_map' instead of 'rl_bind_key'.
> And I guess 'rl_get_keymap_by_name' is the function we should use to get
> the keymap.  From keymaps.h in readline:
>
>   /* Return the keymap corresponding to a given name.  Names look like
>      `emacs' or `emacs-meta' or `vi-insert'.  */
>   extern Keymap rl_get_keymap_by_name PARAMS((const char *));
>
>       Mark
I'm excited to have a look but I have 0 knowledge of the code base (and
am just learning Scheme although that's probably not an issue in this
case whatsoever). Do you have any advice for specific
documentation/files (other than README and HACKING) that I should read
if I were to attempt this? As well as which branch to base my changes onto.

Daniel






^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-24 12:42       ` Daniel Tam
@ 2018-08-24 22:11         ` Mark H Weaver
  0 siblings, 0 replies; 9+ messages in thread
From: Mark H Weaver @ 2018-08-24 22:11 UTC (permalink / raw)
  To: Daniel Tam; +Cc: 32501

Hi Daniel,

Daniel Tam <danooo.tam@gmail.com> writes:
> I'm excited to have a look but I have 0 knowledge of the code base (and
> am just learning Scheme although that's probably not an issue in this
> case whatsoever).

It's okay, please don't feel any obligation to work on this.  Sometimes
people are impatient to fix a problem ASAP, and that's why I wrote these
preliminary emails.

It's probably better for you to focus on learning Scheme for now, rather
than mucking around in the underbelly of an implementation :)

I appreciate you taking the time to file this bug report.  That's quite
useful by itself.

      Thanks!
        Mark





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#32501: Bug #32501 - Bouncing parentheses broken in REPL with vi-mode enabled
  2018-08-22 15:47 bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled Daniel Tam
  2018-08-23  2:42 ` Mark H Weaver
@ 2022-07-11 12:20 ` Sean
  1 sibling, 0 replies; 9+ messages in thread
From: Sean @ 2022-07-11 12:20 UTC (permalink / raw)
  To: 32501

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

   set blink-matching-paren on






Adding this line to your  ~/.inputrc provides this functionality, even with vi-mode enabled.



This bug could be closed, perhaps with a documentation note somewhere.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-07-11 12:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-22 15:47 bug#32501: Bouncing parentheses broken in REPL with vi-mode enabled Daniel Tam
2018-08-23  2:42 ` Mark H Weaver
2018-08-23  4:03   ` Mark H Weaver
2018-08-23 16:13     ` John Cowan
2018-08-24  5:36   ` Mark H Weaver
2018-08-24  6:09     ` Mark H Weaver
2018-08-24 12:42       ` Daniel Tam
2018-08-24 22:11         ` Mark H Weaver
2022-07-11 12:20 ` bug#32501: Bug #32501 - " Sean

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).