allow \cJ (\n) to be bound separate from \cM (\r)

[krader1961]

This makes it possible (on UNIX systems, don't know about MS Windows)
to bind \cJ (\n) independently of \cM (\r, aka [enter]).

Resolves #217

[michaeljones]

👍 compiles and works for me. Very happy indeed! Thanks for taking the time.

function fish_user_key_bindings
    bind \cj history-search-forward
    bind \ck history-search-backward
end

[faho]

Seems to work. I'm guessing there's no way to make \cm work as well?

(To be clear, before \cm, \cj and enter would always do the same thing, after only enter and \cm do)

[krader1961]

I'm guessing there's no way to make \cm work as well?

I don't understand the question. You can bind \cm to anything you want but obviously doing so affects the [enter] key since that is what it sends (assuming you haven't customized your terminal). My change adds bindings for \cm to (aka \r) to the execute function since that provides the expected behavior. My change also binds \cj to execute so it does the same thing as \cm (just in case someone has a terminal configured such that the [enter] key sends \n.

[faho]

I don't understand the question. You can bind \cm to anything you want but obviously doing so affects the [enter] key since that is what it sends

My question was about uncoupling \cm and [enter], since they are quite obviously not the same key on my keyboard. Unfortunately it seems that one's just inherent to the unix terminal model - though I believed that about \cj as well.

[krader1961]

Dedicated terminals like a DEC VT100 or Televideo 925 transmit \cm (aka carriage-return, aka \r) when the [enter] key is pressed. A few of them allowed configuring the specific char that was sent. Most modern terminal emulators like xterm and iTerm2 allow you to configure pretty much any key to be customized with respect to the characters transmitted when the key is pressed. So, yes, you can uncouple \cm and [enter] but that's outside the scope of fish and the UNIX tty driver.

The reason the UNIX tty driver is normally configured to convert all incoming \cm characters to \cj is so that you can interactively enter lines of text into programs like sed. In other words the tty driver makes it look like you're entering lines of text that end in a newline (\n or \cj) character which is what most UNIX commands that process text expect.

[michaeljones]

Hi, I've noticed an issue with my shell when I use the bindings I mentioned above. ie:

function fish_user_key_bindings
    bind \cj history-search-forward
    bind \ck history-search-backward
end

It happens when I'm using tmuxinator which creates & runs a bash script to set up a tmux session with multiple panels.

I've simplified the script to:

#!/bin/bash

tmux start-server\; has-session -t fish-test 2>/dev/null

if [ "$?" -eq 1 ]; then
  cd ~/

  # Create the session and the first window. Manually switch to root
  # directory if required to support tmux < 1.9
  TMUX= tmux new-session -d -s fish-test -n run

  # sleep 3

  # Window "run"
  tmux send-keys -t fish-test:0 echo\ 1 c-m
  tmux send-keys -t fish-test:0 echo\ 2 c-m
  tmux send-keys -t fish-test:0 echo\ 3 c-m
  tmux send-keys -t fish-test:0 echo\ 4 c-m
  tmux send-keys -t fish-test:0 echo\ 5 c-m
  tmux send-keys -t fish-test:0 echo\ 6 c-m
  tmux send-keys -t fish-test:0 echo\ 7 c-m
  tmux send-keys -t fish-test:0 echo\ 8 c-m
  tmux send-keys -t fish-test:0 echo\ 9 c-m
  tmux send-keys -t fish-test:0 echo\ 10 c-m

  tmux select-window -t 0
fi

  if [ -z "$TMUX" ]; then
    tmux -u attach-session -t fish-test
  else
    tmux -u switch-client -t fish-test
  fi

When I run this command without the bindings in place. I get a tmux session which looks like this:

echo 1
echo 2
echo 3
echo 4
$ echo 1
1
$ echo 2
2
$ echo 3
3
$ echo 4
4
$ echo 5
5
$ echo 6
6
$ echo 7
7
$ echo 8
8
$ echo 9
9
$ echo 10
10

Note that all the echo commands are run even if some of the input text groups at the start.

If I run with the above bindings then I get this:

echo 1
echo 2
echo 3
echo 4
echo 5
$ echo 1echo 6
1echo 6
$ echo 7
7
$ echo 8
8
$ echo 9
9
$ echo 10
10
$

ie. some of the echo commands aren't run at all and two of them end up on the same line and then the rest are ok.

I can resolve this locally by uncommentting the 'sleep 3' line in the script which appears to allow everything to set up and go properly.

I'm honestly not sure how to interpret the results. I'm not confident in saying it is definitely an issue with fish or this change but the only difference between the two runs is a change in the fish user bindings.

I'm not sure how to interpret it exactly but is there something in the initialization order of the key bindings and the latest change that might result in a period where the C-m line endings from the tmux send-keys aren't caught properly?

As I said, I can work around it for the moment. Only a small delay is required to make it work but I hope someone with more experience than me might see this pattern and be able to shed some light on the cause.

Cheers,
Michael

[krader1961]

@michaeljones You're observing a race condition. Tmux creates a pty and initializes it with ICRNL mode enabled. It then launches fish on the slave side of the pty. While fish is initializing tmux pushes a bunch of characters into the master side of the pty. Because fish hasn't yet unset ICRNL mode the \cm characters are converted to \cj by the pty driver. While tmux is busy pushing characters into the master side of the pty fish eventually executes the code that unsets ICRNL mode. This causes any subsequent \cm characters to be left as is. You can make the effect even more noticeable by using

bind -m insert \cj backward-delete-char

This has nothing do do with the key bindings (at least not directly). It's a consequence of the pty driver ICRNL mode changing while tmux is pushing \cm characters into the pty. It would be trivial to configure tmux to launch vim with similar bindings and see the exact same problem.

You can "fix" this by downloading the tmux source, applying the following patch, and building a custom tmux binary. Note that "fix" is in quotes. I do not recommend this except to prove my point.

diff --git a/client.c b/client.c
index e24d07e..c9d2290 100644
--- a/client.c
+++ b/client.c
@@ -314,8 +314,9 @@ client_main(struct event_base *base, int argc, char **argv, int flags,        if (client_flags & CLIENT_CONTROLCONTROL) {
                if (tcgetattr(STDIN_FILENO, &saved_tio) != 0)
                        fatal("tcgetattr failed");
+               saved_tio.c_iflag &= ~ICRNL;
                cfmakeraw(&tio);
-               tio.c_iflag = ICRNL|IXANY;
+               tio.c_iflag = IXANY;
                tio.c_oflag = OPOST|ONLCR;
 #ifdef NOKERNINFO
                tio.c_lflag = NOKERNINFO;
diff --git a/cmd-new-session.c b/cmd-new-session.c
index 65dc6cf..1bd8f38 100644
--- a/cmd-new-session.c
+++ b/cmd-new-session.c
@@ -148,6 +148,7 @@ cmd_new_session_exec(struct cmd *self, struct cmd_q *cmdq)
                }
                if (tcgetattr(c->tty.fd, &tio) != 0)
                        fatal("tcgetattr failed");
+               tio.c_iflag &= ~ICRNL;
                tiop = &tio;
        } else
                tiop = NULL;
diff --git a/window.c b/window.c
index 78f944c..f61e3b0 100644
--- a/window.c
+++ b/window.c
@@ -861,6 +861,7 @@ window_pane_spawn(struct window_pane *wp, int argc, char **argv,

                if (tcgetattr(STDIN_FILENO, &tio2) != 0)
                        fatal("tcgetattr failed");
+               tio2.c_iflag &= ~ICRNL;
                if (tio != NULL)
                        memcpy(tio2.c_cc, tio->c_cc, sizeof tio2.c_cc);
                tio2.c_cc[VERASE] = '\177';

[krader1961]

Also, @michaeljones, to clarify my previous reply I should point out that in general you have to add sufficient delay before the "tmux send-keys" statements to allow the program (be that fish, vim, emacs, or something else) to complete its initialization and be ready to accept input. Three seconds is almost certainly an order of magnitude longer than is necessary for reliable behavior. I would expect fish to complete its initialization of the tty input layer within a couple of hundred milliseconds unless running on a horribly overloaded system. Are you not seeing that script work reliably if you reduce the delay to one second? If not then I would be curious about why it's taking fish on your system so long to initialize the tty input layer.

Your problem report also makes me wonder if this "gotcha" shouldn't be documented somewhere such as the FAQ page.

[michaeljones]

@krader1961, thank you for the response. I'm in awe of your knowledge of this stuff.

You are right that the 3 second delay is overkill. Sort of left there for debugging. It works with something much shorter though I haven't honed in on a reliable value yet.

Whilst I follow your explanation and it seems to make sense, I'm curious that I didn't see this issue with zsh. If the shell is not at fault, it seems strange that one would work and another wouldn't? Though I can't see a work around if, as you say, the \cm is already converted to a \cj by the pty driver before it reaches the shell. Maybe zsh somehow priorities unsetting ICRNL as one of the very first things that it does? I have the same binding in zsh.

Another possible cause is that I don't get the colors properly when doing an 'ls' when running fish inside my terminal emulator (termite.) I do get them inside fish inside the tmux session though. I assume this is a term info issue. Perhaps it is possible that when I launch tmux, the shell that I launch it from doesn't have ICRNL set due to some terminfo issue? Again, I'm not wise in these things. I'll try to explore over the weekend. I was ignoring the colors issue as I'm pretty much always in tmux but I'm happy to investigate if it might be related in some way.

[krader1961]

We're now wondering far afield from the purpose of this change. But for the record I used dtruss to time how long it took fish and zsh to issue the first ioctl() syscall that changes the tty driver behavior. The average of six invocations for each shell on my Macbook Pro laptop yielded 332 ms for zsh and 463 ms for fish. Now dtruss greatly slows down a process so those times are much larger than they would be normally. Nonetheless it is obvious that fish is doing a lot more before reconfiguring the tty driver and that is probably something that should be changed if possible. It also shows that you could theoretically see a problem with zsh as it too takes a significant amount of time (and several thousand syscalls) before it too disables ICRNL mode. The incorrect colors are going to be due to an incorrectly set TERM env var and or LS_COLORS env var. It is quite literally impossible for disabling ICRNL mode in the tty driver to be a factor.

[krader1961]

Thank you, @michaeljones, for pointing out that fish takes an awfully long time to reconfigure the tty driver. I've added another change to this pull request that reduces this time by a factor of at least 300. With that change in place I can no longer reproduce the failure mode you reported. Having said that it is important to note that you still theoretically need an explicit short delay (on the order of 0.1 seconds) before the first tmux send-keys because even with my second change there is still an approximately 2 ms delay before the tty is reconfigured. That's really, really, tiny and unlikely to have any noticeable problems in practice but still allows for a race condition if tmux is super fast in stuffing those characters into the pty.

[michaeljones]

@krader1961, very happy to be of help. Thank you very much for addressing these things so quickly and for your patience in explaining the various details to me. Sorry for the terminfo digression I was clutching at straws.

I've compiled this additional change and can confirm that my tmux sessions load perfectly now without the need for artificial delays. Thanks again!

[krader1961]

I was simply following the behavior of the existing code, @ridiculousfish, which unconditionally calls reader_init() from main. If stdin is not associated with a tty then the ioctl syscalls used by tcgetattr and tcsetattr calls harmlessly fail with errno == ENODEV (or possibly ENOTTY). If the shell is in the background the tcgetattr will succeed but the tcsetattr will harmlessly fail (although I don't recall off the top of my head what the errno will be). All of this code should be predicated on either the is_interactive_session variable or a isatty() call but that is a far larger change with a greater chance of breaking existing behavior so I'm disinclined to do so as part of resolving this specific issue.

[ridiculousfish]

Ok, let's do what you did originally then. We restore the terminal process group on exit anyways.

Merged as 16c34b3. Thanks! Great comments!!