Re: [Nano-devel] [PATCH] add rebinding for Ctrl+Arrow-keys combinations

[Benno Schulenberg]

Hello Rishabh,

On Tue, Aug 2, 2016, at 21:51, Rishabh Dave wrote:
> I have tried to resolve "bug #44688: rebinding ^Left and ^Right does
> not work". Link to it - http://savannah.gnu.org/bugs/?44688.

You might have wanted to discuss the bug first.  Maybe the fix is not
to make the arrow keys rebindable but to properly warn that they are
unrebindable (instead of making it seem as if they were rebound but
not actually having done anything, as it is now).

But okay, let's assume that we want to make them rebindable -- maybe
someone wants to assign Undo and Redo to ^Left and ^Right, and Find-
Previous and Find-Next to ^Up and ^Down.

> Since I was dubious regarding my method, I have kept the patch
> incomplete (checking for and updating code to new commits,

Ah, but /that/ you should do.  A patch against old code is not useful.

About the patch itself:

     } else /* RAWINPUT */
        s->seq = (int) s->keystr[0]

Those lines don't exist any more.  Patch fails to apply.

+    if (!strcasecmp(s->keystr, "^Left"))
+       s->seq = 544;

??  544?  Where did you get that value from?  And how do you know
it will have that value everywhere?  There are defines for those
things; use them.

+       /* Find same function in sclist and assign same toggle value
+        * to new shortcut. */
+       newsc->toggle = first_sc_for(currmenu, newsc->scfunc)->toggle;

What are those lines doing there?  Are you saying that the rebinding
of toggles doesn't work right now?

-                           case 'C':
-                               retval = controlright;
-                               break;
-                           case 'D':
-                               retval = controlleft;
-                               break;

You are making ^Left and ^Right completely nonfunctional on an iTerm2.

-       if (retval == controlleft)
-           retval = sc_seq_or(do_prev_word_void, 0);
-       else if (retval == controlright)
-           retval = sc_seq_or(do_next_word_void, 0);

Same thing for any terminal where ^Left isn't 544 and ^Right isn't 559.
If anything, you should do this:

        if (retval == controlleft)
-           retval = sc_seq_or(do_prev_word_void, 0);
+           retval = CONTROL_LEFT;
        else if (retval == controlright)
-           retval = sc_seq_or(do_next_word_void, 0);
+           retval = CONTROL_RIGHT;

Benno

-- 
http://www.fastmail.com - A fast, anti-spam email service.