Term: Line/Char gets out of sync with insert state

[emacs-evil]

Term: Line/Char gets out of sync with insert state

[Ambrevar]

I've had issue with term but I can't remember that one. I'll keep that in mind and try to reproduce.

[jojojames]

ffa7e1a might have fixed it. I'll close for now and reopen if I continue seeing it.

[jojojames]

Actually the problem is in this function.

(defun evil-term-char-mode-entry-function ()
  "Maybe switch to `term-char-mode' on insert state."
  (when (get-buffer-process (current-buffer))
    (let (last-prompt)
      (save-excursion
        (goto-char (point-max))
        (when (= (line-beginning-position) (line-end-position))
          (ignore-errors (backward-char)))
        (setq last-prompt (max (term-bol nil) (line-beginning-position))))
      (when (>= (point) last-prompt)
        (term-char-mode)))))

When the term buffer grows past the prompt (by one or two lines), this function always fails the >= so it will never call term-char-mode.

[Ambrevar]

I don't understand the issue, maybe you are expecting a different behaviour. I wrote this code so that going to insert mode when point is on the last line and after the prompt will switch to char mode.
Because terminals can only edit the last part, i.e. the stuff after the prompt on the last line. You cannot be in char mode above the last line if I'm not mistaken.

[jojojames]

The assumption is shaky because the buffer can grow past the last prompt making the boolean condition always false. (i.e. your last-prompt check wlil always be bigger than (point)).

You can get into this state with the cursor on the same line as the last prompt and never go back to char mode.

[Ambrevar]

The buffer is always "past the last prompt", so I don't understand what you mean.

Can you give an example?

[jojojames]

Assume the buffer grows past the last prompt by (1 or 2 or 3 lines). The point check will always fail then.

Good state:
https://www.dropbox.com/s/gx0p7eh1fe1jqq3/Screenshot%202017-11-28%2009.16.57.png?dl=0

Bad state:
https://www.dropbox.com/s/0pkzqyafuu2io4h/Screenshot%202017-11-28%2009.25.30.png?dl=0

Try pressing RET in normal state after typing 'ls' in insert state a few times as one way to repro.

[Ambrevar]

Sorry for the delay, I forgot about this.

I cannot reproduce though. I still do not understand what you mean with "buffer grows past the last prompt".
Looks like we are on a completely different page. Can you explain differently?

[jojojames]

Did you take a look at the two pictures? Note the cursor position in both. In the 'bad' state, the cursor is wayy at the bottom of the buffer. That's because the buffer grew past the prompt.

Here's a video.

https://www.dropbox.com/s/6sequ83b0n1lj1l/1.mov?dl=0

[Ambrevar]

Thanks for the video, it shows a few things the pics did not show.
First you seem to suffer from a "lag" when you keep pressing RET. It generates empty lines between prompts.
I never get this behaviour. That might be a clue, but new lines are not an issue for me either.

I tried reproducing from a minimal config:

(require 'evil)
(evil-mode 1)
(require 'evil-collection)
(evil-collection-init)

to no avail. It desperately works for me :)

Try a minimal config. If you can reproduce, then the difference might lie in the shell / prompt.

• Shell: bash.
• Prompt: john@doe:/path/$
• term-prompt-regexp: "^"
• Emacs version: 25.3.

That's because the buffer grew past the prompt.

I'm still confused by your terms so let's lay down some definitions to be sure we are on the same page.

• Prompt: john@doe:/path/$

• Buffer: all the "character" in the window, from the first prompt to the last commandline and its output. Everything.

To me, when you say "grow past the prompt", I understand that (point-max) is after the last prompt, which is a tautology, since the prompt is part of the buffer.

Maybe you meant that there are new lines inserted after the last prompt.
Even then, the code should not fail.

On my end, if I insert N newlines with C-q C-j in line-mode and then press RET (term-send-input), the underlying shell will properly receive the N newlines. From there, going to insert mode will effectively switch to char mode.

[jojojames]

I think you are looking too closely at the 'repro' instead of the outcome. What's in the video is just one way to reproduce this bug. The bug itself is in the flawed check, (if the buffer grows past the prompt by more than a few lines, the check will always fail).

Things like 'wrong shell', 'wrong setup' doesn't seem to be right when the check for switching to insert/normal state is what's flawed.

To me, when you say "grow past the prompt", I understand that (point-max) is after the last prompt, which is a tautology, since the prompt is part of the buffer.

Again, the special qualifier is 'by a couple of lines'.

On my end, if I insert N newlines with C-q C-j in line-mode and then press RET (term-send-input), the underlying shell will properly receive the N newlines. From there, going to insert mode will effectively switch to char mode.

I would expect this to work. (i.e. I do not expect this to reproduce the bug).

I think a good route going forward is just to disable this kind of 'smart' code (or at least provide the option to) as it's obvious it won't work for a variety of setups.

[Ambrevar]

Why would (>= (point) last-prompt) be nil after a "couple of lines"?

[jojojames]

Err lets disect the function then...

(defun evil-collection-term-char-mode-entry-function ()
  "Maybe switch to `term-char-mode' on insert state."
  (when (get-buffer-process (current-buffer))
    (let (last-prompt)
      (save-excursion
        (goto-char (point-max))
        (when (= (line-beginning-position) (line-end-position))
          (ignore-errors (backward-char)))
        (setq last-prompt (max (term-bol nil) (line-beginning-position))))
      (when (>= (point) last-prompt)
        (term-char-mode)))))

(goto-char (point-max)) --> What happens if the buffer is extremely large, relative to the prompt (as shown in both screenshots and video?)

(setq last-prompt (max (term-bol nil) (line-beginning-position)))) --> What will last-prompt be if point-max was large?

[Ambrevar]

(goto-char (point-max)) --> What happens if the buffer is extremely large, relative to the prompt (as shown in both screenshots and video?)

It goes to point-max. Size does not matter here, does it?

If there was an issue in the line's logic, you should be able to reproduce with just one extra line. I don't understand why "extreme" amounts are necessary to reproduce the issue.

(setq last-prompt (max (term-bol nil) (line-beginning-position)))) --> What will last-prompt be if point-max was large?

It will be (line-beginning-position).

Since you cannot edit what's before the last line in char-mode, then if (point) is before the last line's (line-beginning-position) it will not switch to char-mode.

That "lag" in your terminal makes me realize there could be a race condition however: if the buffer keeps growing while the code is executed, then it will fail to switch to char-mode as it should.
That is, if more newlines are inserted between the time

 (goto-char (point-max))

and

(setq last-prompt (max (term-bol nil) (line-beginning-position))))

are executed. Then indeed the condition will be wrong.

Not sure how to solve that.

[Ambrevar]

Do you know why your terminal is so slow by the way? Some fancy prompt calculation?

Could you try with /bin/sh just to confirm my hypothesis?

[Ambrevar]

Correction: the race condition is between

        (setq last-prompt (max (term-bol nil) (line-beginning-position))))

and

      (when (>= (point) last-prompt)

[jojojames]

Yes bin/sh is fast enough so that the repro steps above do not work but again, that's just one way to repro it.

Is race condition the correct categorization? After you've entered in that state, it will fail everytime.

Do you know why your terminal is so slow by the way? Some fancy prompt calculation?

That or zsh is slow or osx terminal is slow.

[Ambrevar]

After you've entered in that state, it will fail everytime.

Yeah? We need to understand precisely why if we want to fix this.

If I get it properly, you are stuck in line-mode correct?

Can you edebug what happens if you wait for output to stabilize, go to the end of the buffer and switch to insert mode (e.g. by pressingGA)?

[jojojames]

Yeah? We need to understand precisely why if we want to fix this.

I'm not sure I grok this. Do you want to understand why we get to that state or do you want to understand why the check fails after we've reached the 'bad' state?

The first is irrelevent. The second is obvious (in my opinion).

I feel like my previous comments already have a good amount of information there.

If I get it properly, you are stuck in line-mode correct?

Yes, as the video shows, going back to insert state will not go back to char-mode.

It will be (line-beginning-position).
Since you cannot edit what's before the last line in char-mode, then if (point) is before the last line's (line-beginning-position) it will not switch to char-mode.

That's the problem. line-beginning-position will be bigger than (point).

Can you edebug what happens if you wait for output to stabilize, go to the end of the buffer and switch to insert mode (e.g. by pressingGA)?

GA switches it back to char-mode and resets the whole thing back to a good state.

[Ambrevar]

GA switches it back to char-mode and resets the whole thing back to a good state.

Doesn't it contradict your previous points, i.e. that you cannot go back to char-mode?

I feel like you are expecting a different behaviour than me, hence the on-going misunderstanding since the beginning... But we are getting there ;)

I think I see what's going on: when you are in line-mode, you type some command and press RET. At this point, you want to enter insert mode, but because of the aforementioned race-condition, you end up in line-mode at line N while the buffer last line is N+P.

If you don't move to the last line, you won't be able to enter char-mode, and that's term expected behaviour: it does not make sense to be in char-mode on anything but the last line.

Note that this synchronization between line-mode/normal-state and char-mode/insert-state is an additional feature, you can still use C-c C-k and C-c C-l as in Emacs state. Nothing is really broken here.
Well, term is a bit broken itself, but that we should work out with upstream.

I'll think of a fix.

[jojojames]

Doesn't it contradict your previous points, i.e. that you cannot go back to char-mode?

No, it doesn't. Typing 'GA' to go back to char-mode is what I'd consider a 'workaround', but I could also just run the command to switch me back to char-mode (what I've been doing). It's like saying "well you can open another terminal and you'd be back in char-mode so there's no bug!" Sorry, just have to drive this point down. :)

I feel like you are expecting a different behaviour than me, hence the on-going misunderstanding since the beginning... But we are getting there ;)

Are you sure? I have a video that shows the bug in action.

I think I see what's going on: when you are in line-mode, you type some command and press RET. At this point, you want to enter insert mode, but because of the aforementioned race-condition, you end up in line-mode at line N while the buffer last line is N+P.

Again, that's looking at what reproduces the problem and not the problem.

If you don't move to the last line, you won't be able to enter char-mode, and that's term expected behaviour: it does not make sense to be in char-mode on anything but the last line.

last line is not correct in this case. last line is more like 'If you don't move to the prompt line, you won't be able to enter char-mode', because as you see in the repro, you are on the last line when the bug repros.

I think this reply might be muddy so I'll re-iterate. 'Last line' is what I'd consider the buffer's last line but I think your 'Last line' is where the last Prompt is.

Note that this synchronization between line-mode/normal-state and char-mode/insert-state is an additional feature, you can still use C-c C-k and C-c C-l as in Emacs state. Nothing is really broken here.
Well, term is a bit broken itself, but that we should work out with upstream.

I have a problem with this characterization because it is broken. This isn't an additional feature because it swallows up the term experience. Without this feature, I would never go to line-mode unless I manually do it. Since this feature is toggling me between the two, and because this bug exists, it enters line-mode unnecessarily (understatement).

Without this feature, I would be in char-mode 99% of the time, so you can see my frustration :) with it now going to line-mode and bugging out in line-mode.

Without a fix, it will probably be better just to not be too smart in this case and let the user manually select the mode as they did before and default to char-mode at the start.

[Ambrevar]

What I mean with "last line" is the buffer's last line. No misunderstanding here.

because as you see in the repro, you are on the last line when the bug repros.

I think I know what's happening: the current line might end with \n which prevents you from going down, thus you might think you are on the last line while you are not.

        (when (= (line-beginning-position) (line-end-position))
          (ignore-errors (backward-char)))

fixes that problem but it will fail in case of a race condition.
This is why, I think, GA works for you while i on the last-line-ending-with-\n does not.

Allow me to explain the intention of that "feature".
Say my feature is not enabled and we use Evil.
Let's fire up a term in char-mode and write:

> ls -- foo bar

Only then we realize we want to add some flags to ls, so we press ESC and move the point after ls to insert:

> ls -l -- foo bar

Now we press RET and the following command actually gets passed to the underlying shell:

ls -- foo bar -l

Do you see why? Because the movement keys in normal mode cannot send corresponding keys to the shell.

That makes the whole experience very frustrating for Evil users: we basically cannot press "ESC".
It's not so much the case in Emacs state because most shells support Emacs-style bindings by default.

With my feature, it will make sure that going to insert state will enter char-mode and move the point back to where we left: not ideal, but at least the user will not type the wrong command.

Note that I had been working on a huge pile of code to properly synchronize the normal-state/line-mode and the underlying shell by moving the cursor manually.
It kind of works but that code suffers from those "race conditions" so badly that it's barely usable.

[Ambrevar]

Just to be clear: using /bin/sh, does it work as expected for you?

[Ambrevar]

Unless my last hypothesis is wrong, I think now I understand the whole extend of the issue: in some conditions (e.g. slow shells) the sync will hit a race condition. If we cannot work around this (for now I don't see how) then I propose to make this an option (disabled by default).

[jojojames]

Allow me to explain the intention of that "feature".

I understand the feature. I am saying that it has a bug.

With my feature, it will make sure that going to insert state will enter char-mode and move the point back to where we left: not ideal, but at least the user will not type the wrong command.

Honestly, it's just easier to get char-mode working. I'm fine with just using the emacs bindings here. (e.g. What you'd get in a normal terminal.)

Note that I had been working on a huge pile of code to properly synchronize the normal-state/line-mode and the underlying shell by moving the cursor manually.
It kind of works but that code suffers from those "race conditions" so badly that it's barely usable.

Yes, I don't think we'll be able to get this feature stable enough. At this point I consider it fairly experimental.

Unless my last hypothesis is wrong, I think now I understand the whole extend of the issue: in some conditions (e.g. slow shells) the sync will hit a race condition. If we cannot work around this (for now I don't see how) then I propose to make this an option (disabled by default).

It seems like we're on the same page. Lets make this optional and default it to false. I'm okay with defaulting this to enabled by default if it were working 100% but I don't think playing around with the Term Prompt is gonna yield 100% of the time.

[Ambrevar]

Good! Glad we finally sorted this out! :D

Honestly, it's just easier to get char-mode working.

Can you be more specific? What features would you be looking for?

At last: without that syncing, what does evil-term do? It has almost nothing Evil to it, or does it? :p

[Ambrevar]

a1ad79a made the sync optional.

Feel free to close this if there is nothing more to change or discuss.

[jojojames]

Can you be more specific? What features would you be looking for?

I had term integration before but we dropped it in favor of your version. To be honest, since mine didn't have the buggy behavior, anything like that was fine. I think your change makes it like that now so that's good. Thanks!

At last: without that syncing, what does evil-term do? It has almost nothing Evil to it, or does it? :p

It does what all the other integration files we have does, sets up bindings in a reasonable fashion.
Insert and normal state have keybindings on them. I'm satisfied with that.

Thanks, I'll close this and revisit if I find something off.

[jojojames]

@Ambrevar I noticed in recent versions of emacs 26 that you can't move out of the prompt in char-mode. So I'd vote to move to making your stuff the default (even if it's a little buggy :))

Maybe we can revisit making the integration tighter at some point.

[Ambrevar]

What do you mean? You can't move "out of the prompt" in char-mode on Emacs 25 either.

[jojojames]

I will double check, but I believe that is not true.

It was a recent change on emacs-26 branch that changed this behavior.

[jojojames]

Had a chance to look at Emacs 25. The behavior is different between the two platforms and it was a recent change on Emacs 26 that changed the behavior. The change was intended though (won't change back to the old Emacs 25 behavior), so I'll change the default back to t.

Feel free to change it back to nil if you feel that's a better default @Ambrevar.

I'll still leave this open since it'd be nice if we could make it 100% bulletproof.

[jojojames]

Here's the relevant NEWS entry.

*** `term-char-mode' now makes its buffer read-only.

The buffer is made read-only to prevent changes from being made by
anything other than the process filter; and movements of point away
from the process mark are counter-acted so that the cursor is in the
correct position after each command.  This is needed to avoid states
which are inconsistent with the state of the terminal understood by
the inferior process.

New user options `term-char-mode-buffer-read-only' and
`term-char-mode-point-at-process-mark' control these behaviors, and
are non-nil by default.  Customize these options to nil if you want
the previous behavior.

[Ambrevar]

I'll try it out as soon as possible.

From what I read there, it sounds to me that term can now be fully sync'ed with it's underlying shell, so we can effectively synchronize normal/insert state with line/char mode.

Do you still experience the issue you mentioned? If so, we might have to do something more on our end.

[jojojames]

I'm keeping a watch on it. I haven't seen it recently so things are looking good.

Once I have some time, I'll try to repro with the same steps as described above.

[jojojames]

• I still see ^ even with the new emacs 26 since my terminal prompt is slow enough.

@Ambrevar Do you have use cases for where you don't want to enter term-char-mode when you go into insert mode?

I'm thinking we can support an option (maybe as a default) to do something like this instead.

(defun evil-collection-term-sync-state-and-mode ()
  "Sync `term-char-mode' and `term-line-mode' with insert and normal state."
  (add-hook 'evil-insert-state-entry-hook 'term-char-mode nil t)
  (add-hook 'evil-insert-state-exit-hook 'term-line-mode nil t))

I generally want to go back to the prompt if I go into insert state (as far as I can think of but I might be missing some important use cases) so this works fine for me.

[Ambrevar]

The use case would be as for Eshell: to allow direct editing of the output, which is a very nice feature in my opinion. On the one hand, it violates the integrity of the output, on the other hand, it saves some copy-pasting.

We could definitely make an option between the current implementation and your suggestion.

[jojojames]

Hmnn OK, I added an option for it (as well as made it the default). If you think the original function should be the default, feel free to set it back. Either works for me. :)