Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CTRL-P/CTRL-N unexpected behavior #78

Open
doug-moen opened this issue Jul 15, 2020 · 5 comments
Open

CTRL-P/CTRL-N unexpected behavior #78

doug-moen opened this issue Jul 15, 2020 · 5 comments

Comments

@doug-moen
Copy link
Contributor

CTRL-P and CTRL-N do not work as expected in the head revision.
What they should do is move up and down the history list, just like up-arrow and down-arrow.
They used to work this way, but the behaviour has changed.

Can reproduce using the example application.

@doug-moen
Copy link
Contributor Author

The problem with this new behaviour is that it conflicts with the defacto standard for how CTRL-P and CTRL-N work in all other apps that I've tested: eg, the bash shell, the python shell, etc. I guess what I want is readline compatibility with no surprises for my users.

If you type CTRL-P on an empty line, instead of getting the previous history item, as expected, you get the last identifier in the completion list, which isn't useful in this context.

I found the following code:

    bind_key( Replxx::KEY::control( 'N' ),                 _namedActions.at( action_names::COMPLETE_NEXT ) );
    bind_key( Replxx::KEY::control( 'P' ),                 _namedActions.at( action_names::COMPLETE_PREVIOUS ) );

I guess the new behaviour is deliberate.

The above code conflicts with the comments in this code:

// ctrl-N, recall next line in history
Replxx::ACTION_RESULT Replxx::ReplxxImpl::history_next( char32_t ) {
       return ( history_move( false ) );
}
// ctrl-P, recall previous line in history
Replxx::ACTION_RESULT Replxx::ReplxxImpl::history_previous( char32_t ) {
      return ( history_move( true ) );
}

@AmokHuginnsson
Copy link
Owner

AmokHuginnsson commented Jul 16, 2020

Hi @doug-moen

Yes. In fact CTRL-P and CTRL-N mappings were deliberately changed from readline's defaults.
The old behavior can be brought back in client application by adding

bind_key_internal( Replxx::KEY::control( 'N' ), "history_next" );
bind_key_internal( Replxx::KEY::control( 'P' ), "history_previous" );

lines.

The discussion wether changing the default was a good decision in the long term would be interesting
but possibly off-topic at the same time.

I hope that new API that replxx provides makes it easy for every client to get back to old behavior.

@AmokHuginnsson
Copy link
Owner

Sorry, for closing this issue, that was a miss-click.

@doug-moen
Copy link
Contributor Author

You may have changed other key bindings to be different from readline, or you may decide to make additional changes in the future.

What would help me is a standard function to reset the key bindings to be readine compatible:

void Replxx::use_readline_key_bindings()

Would you consider adding this function?

@AmokHuginnsson
Copy link
Owner

AmokHuginnsson commented Jul 16, 2020

replxx could read configuration file in the vein of .inputrc.
This configuration file could contain this kind of bash-compatible settings.
I would accept pull request implementing this kind of feature for replxx,
I mean loading replxx initialization file (could be loaded from ${HOME}/.replxxrc by default or from an explicit path).

Personally I will work on adding support for 256 color palette to replxx first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants