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

No way to input single "ctrl-p" keypress #4208

Closed
cjao opened this issue Oct 6, 2019 · 18 comments
Closed

No way to input single "ctrl-p" keypress #4208

cjao opened this issue Oct 6, 2019 · 18 comments
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue

Comments

@cjao
Copy link

cjao commented Oct 6, 2019

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

BUG

Description

While inside a terminal inside a container (such as toolbox), pressing "ctrl-p" does nothing while pressing it again sends two "ctrl-p" events to the terminal. This behavior interferes with the standard keybindings of common programs like emacs (where ctrl-p normally scrolls up one line) or bash (where ctrl-p steps back in the command history). Here is a simple demonstration of the problem.

  1. podman run --rm -ti fedora-toolbox:30 bash

  2. echo 1

  3. echo 2

  4. echo 3

  5. Observe that "ctrl-p" does nothing while pressing it again returns echo 2 instead of echo 3.

Alternatively, check that using "ctrl-p" only allows one to scroll up two lines at a time in emacs.

One workaround would be to change the "detach-keys" sequence from "ctrl-p,ctrl-q" to something that hopefully does not conflict with any other program. But it would be better to just send one "ctrl-p" event for every two invocations of "ctrl-p".

Additional info:

  • Running Fedora 30
  • rpm -q podman yields
podman-1.6.1-2.fc30.x86_64
@rhatdan
Copy link
Member

rhatdan commented Oct 7, 2019

@haircommander @giuseppe @mheon Is this conmon/podman waiting for a sequence to disconnect from the container?

man podman run
...
       When  attached  in the tty mode, you can detach from the container (and
       leave it running)  using  a  configurable  key  sequence.  The  default
       sequence  is  ctrl-p,ctrl-q.   Configure  the  keys  sequence using the
       --detach-keys option, or specifying it in  the  libpod.conf  file:  see
       libpod.conf(5) for more information.

We might need to either disable this feature or setup alternative key mappings.
I agree the ctrl-p is common for use within bash/emacs settings.

@rhatdan
Copy link
Member

rhatdan commented Oct 7, 2019

@debarshiray WDYT?

@mheon
Copy link
Member

mheon commented Oct 7, 2019

This is the CopyDetachable code in Podman

@rhatdan
Copy link
Member

rhatdan commented Oct 7, 2019

Is there an easy way for toolbox to disable this?

@mheon
Copy link
Member

mheon commented Oct 7, 2019

Reading the code right now: it seems like it pauses the copy until it can check the buffer fully for C-p C-q if a C-p is detected. Once it can check, if it is not that key sequence, it's copied in its entirely - so sending C-p C-a for example would result in both keys being send.

The difficulty with special-casing C-p C-p is that we don't guarantee that the detach sequence is two keys only. We support, say, C-p C-q C-r as a detach sequence - for that case, would we expect that C-p C-p C-p sends a single C-p?

@mheon
Copy link
Member

mheon commented Oct 7, 2019

Actually, the loop would probably terminate early, in the case of 3 keys - so C-p C-p would still work.

@rhatdan
Copy link
Member

rhatdan commented Oct 7, 2019

The think is, how would I run podman and tell it I don't intend to detach. Another would be to put a strange squence. Could toolbox specifiy alt-; or something like that, so it would not be a common sequence used for bash recall.

@mheon
Copy link
Member

mheon commented Oct 7, 2019

I think that setting detach keys to "" (empty string) in libpod.conf will get you no detach keys. That has its own disadvantages, in that there's no way to escape an attach session.

@rhatdan
Copy link
Member

rhatdan commented Oct 7, 2019

Sure, But can toolbox just execute

podman run --detach-keys ""
to do the same.

I don't think toolbox expects people to detach from toolboxes. But I will let @debarshiray comment.

@debarshiray
Copy link
Member

debarshiray commented Oct 7, 2019

Wow, I use ctrl+p in Emacs and Bash all the time but I never noticed this before today. I guess I just don't spent enough quality time inside a Toolbox shell. :)

Yes, I agree. We don't expect people to want to detach a container while running an interactive shell.

@mheon
Copy link
Member

mheon commented Oct 7, 2019

For now, simplest solution would be for Toolbox to override detach keys, then. I don't know if exec allows this yet; we can add it if it doesn't.

@haircommander
Copy link
Collaborator

exec does allow this!

@debarshiray
Copy link
Member

It doesn't look like --detach-keys "" unsets the shortcut. I tried:

$ podman run --env TERM=$TERM --detach-keys "" -it --rm registry.fedoraproject.org/f31/fedora-toolbox:31 /bin/bash

I also tried --detach-keys=.

@debarshiray
Copy link
Member

Grepping around in the code, I found:

DefaultDetachKeys = "ctrl-p,ctrl-q"

func processDetachKeys(keys string) ([]byte, error) {
	// Check the validity of the provided keys first
	if len(keys) == 0 {
		keys = DefaultDetachKeys
	}
	detachKeys, err := term.ToBytes(keys)
	if err != nil {
		return nil, errors.Wrapf(err, "invalid detach keys")
	}
	return detachKeys, nil
}

So, it doesn't look like --detach-keys "" unsets the shortcut, at least not currently.

@rhatdan
Copy link
Member

rhatdan commented Oct 15, 2019

Yes lets open a PR to change the DefaultDetatchKeys to be set at a higher level and then to return empty list with ""

@stove-panini
Copy link

Hey all, any progress on this? @rhatdan do you have a link to the PR you mentioned?

@github-actions
Copy link

This issue had no activity for 30 days. In the absence of activity or the "do-not-close" label, the issue will be automatically closed within 7 days.

@rhatdan
Copy link
Member

rhatdan commented Nov 29, 2019

@stove-panini Looks like this is fixed in upstream, although we have another issue.

@rhatdan rhatdan closed this as completed Nov 29, 2019
debarshiray pushed a commit to HarryMichal/toolbox that referenced this issue Sep 2, 2020
Podman sets 'ctrl-p ctrl-q' as the default key sequence for detaching
a container. This breaks the ctrl-p shortcut that's equivalent to the
up arrow key in GNU Readline environments like Bash and Emacs.
Moreoever, toolbox containers aren't meant to be detached in the first
place.

Since Podman 1.8.1, it is now possible to unset the key sequence for
detaching [2, 3].

[0] https://tiswww.cwru.edu/php/chet/readline/readline.html#SEC15

[1] https://www.gnu.org/software/emacs/tour/

[2] Podman commit 7c623bd41ff3d534
    containers/podman#4208

[3] Podman commit ebfd253fc658ffc9
    containers/podman#5166

containers#394
likan999 pushed a commit to likan999/ppa-toolbox that referenced this issue Oct 30, 2020
Podman sets 'ctrl-p ctrl-q' as the default key sequence for detaching
a container. This breaks the ctrl-p shortcut that's equivalent to the
up arrow key in GNU Readline environments like Bash and Emacs.
Moreoever, toolbox containers aren't meant to be detached in the first
place.

Since Podman 1.8.1, it is now possible to unset the key sequence for
detaching [2, 3].

[0] https://tiswww.cwru.edu/php/chet/readline/readline.html#SEC15

[1] https://www.gnu.org/software/emacs/tour/

[2] Podman commit 7c623bd41ff3d534
    containers/podman#4208

[3] Podman commit ebfd253fc658ffc9
    containers/podman#5166

containers#394
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue
Projects
None yet
Development

No branches or pull requests

6 participants