Command with no output and a non-zero exit code breaks terminal tab.

[Kynolin]

Windows 10: 1903, 1803
WSL Distros: Ubuntu, Artix
Shells: fish, bash

Extraterm 0.46

A command with no output and a non-zero exit code replaces the line where the command was typed with a frame. The content in the tab stops responding to input. Nothing can be typed or displayed, and right click doesn't open up the context menu. Attempting to blindly type commands does not seem to actually do anything if checking for processes in another tab. Removing the frame works, but the tab remains unresponsive to other input.

Edit: This still happens when there is output from the command (either stdout/stderr). After the command with the non-zero exit code, pressing enter on an empty line produces the error. If other commands are typed after the non-zero command, the shell keeps working.

Extraterm 0.45

The frame is displayed, and pressing enter on an empty line frames the empty line itself instead of just printing the next empty prompt in the shell. The tab still functions otherwise.

Screenshot from 0.46:

Screenshot from 0.45:

[Kynolin]

I'm not super familiar with the code, but I finally got around to running it from source. I've narrowed down the breaking issue. Here's where the isEmpty check is failing for me. For some reason my empty command has a range.end.row value of -1.

For example purposes only (since I gave no thought as to what else it could impact), I threw in a quick check to see where that would get me. The terminal no longer breaks completely, and it's back to showing a bunch of empty frames when I press enter (since the return code is still 1) like in version 0.45.

src/TerminalDocument.ts:

const isEmpty = range.start.row === range.end.row && range.start.column === range.end.column;
const shouldBeEmpty = range.start.row === -1 || range.start.column === -1 || range.end.row === -1 || range.end.column === -1;
if ((newText.length === 0 && isEmpty) || shouldBeEmpty) {

I haven't quite narrowed down the repeating, empty frames when pressing enter after the last command exited non-zero, but I'm thinking it will be around here.

src/render_process/Terminal.ts:1155

if (returnCode === "0" && ! this._commandNeedsFrame(this._lastCommandLine, moveTextLines.length)) {

After running a successful command, pressing enter goes to a new line, keeping the previous line, but when the last command was non-zero, it removes it and tries to frame it, even when there's no command, output, or status code. There should be some condition to skip this and other areas to get an empty command to behave the same as it does when the last exit code was zero.

src/render_process/Terminal.ts:1172 (removes the empty command from the screen)

this._lastCommandTerminalViewer.deleteLines(this._lastCommandTerminalLine);
this._lastCommandTerminalViewer = null;

[Kynolin]

So far, this has been the simplest solution that resolves most of the issues I've been having. Non-zero commands follow the normal framing rules, and the moveTextLines index is checked to avoid deleting the command line itself on a command with no output, like false.

I'm still having framing issues after running the clear command, but overall my terminal is much more reliable. I haven't yet tried running the Extraterm tests with my changes.

--- a/extraterm/src/render_process/Terminal.ts
+++ b/extraterm/src/render_process/Terminal.ts
@@ -1152,7 +1152,9 @@ export class EtTerminal extends ThemeableElementBase implements AcceptsKeybindin
       }

       let moveTextLines = this._lastCommandTerminalViewer.getTerminalLinesToEnd(this._lastCommandTerminalLine);
-      if (returnCode === "0" && ! this._commandNeedsFrame(this._lastCommandLine, moveTextLines.length)) {
+      // TODO: Add settings to always frame zero and/or non-zero returnCode.
+      //       Until then, respect framing rules on all return codes.
+      if (! this._commandNeedsFrame(this._lastCommandLine, moveTextLines.length)) {
         // No need to frame anything.
         this._lastCommandLine = null;
         this._lastCommandTerminalViewer = null;
@@ -1169,7 +1171,10 @@ export class EtTerminal extends ThemeableElementBase implements AcceptsKeybindin
       this._disconnectActiveTerminalViewer();

       // Extract the output of the failed command.
-      this._lastCommandTerminalViewer.deleteLines(this._lastCommandTerminalLine);
+      if (moveTextLines.length > 0) {
+        // Only delete lines of output, not the command itself.
+        this._lastCommandTerminalViewer.deleteLines(this._lastCommandTerminalLine);
+      }
       this._lastCommandTerminalViewer = null;

       // Append our new embedded viewer.
@@ -1187,7 +1192,7 @@ export class EtTerminal extends ThemeableElementBase implements AcceptsKeybindin
       outputTerminalViewer.setReturnCode(returnCode);
       outputTerminalViewer.setCommandLine(this._lastCommandLine);
       outputTerminalViewer.setUseVPad(false);
-      if (moveTextLines !== null) {
+      if (moveTextLines.length > 0) {
         outputTerminalViewer.setTerminalLines(moveTextLines);
       }
       outputTerminalViewer.setEditable(true);

I would imagine it would be good to have an option to select if good/bad commands always get framed. For example: if someone wanted to easily notice failed commands, always frame non-zero. In that case, ignoring the returnCode for empty commands would keep it from being too obnoxious.

[sedwards2009]

I think I've seen this occur to me a few times but certainly didn't know the conditions needed to reproduce the problem. Thanks for investigating it. I'll try to get this sorted out by the next release.

[Kynolin]

I just want to add a couple of observations after using 0.46 with my modifications. This is while framing commands if output is longer than 1 line. Side note, that setting seems to frame commands if output is >= 1 line.

After running the clear command, my commands don't get framed initially, but after running additional commands with enough output to fill the screen, commands start being framed again. When my commands are being framed, reset doesn't clear the screen. It just goes to the next line like a command without output would. But, when I've got the framing issue after running clear, reset causes the clear command's full screen of blank output to display in a frame, then subsequent commands are framed successfully.

In fish, running a for loop to echo 1 through 5, with each number on a new line and a sleep after the echo, the output is displayed in-line as if it isn't going to be framed. It then gets removed and framed when the command finishes. On bash, 1-4 display in-line without a frame, then a frame is displayed containing only the number 5, with the previous non-framed lines still displayed. If you take away the sleep, fish frames all the output and bash doesn't display a frame. I first noticed this type of behavior running ssh-copy-id. Unlike the for loop with echo, both fish and bash display the ssh-copy-id output without a frame, then the text is removed and all output displays in one frame.

These were the commands I used:

for i in (seq 1 5); echo $i; sleep 1; end
for i in (seq 1 5); echo $i; end

for i in $(seq 1 5); do echo $i; sleep 1; done
for i in $(seq 1 5); do echo $i; done

[sedwards2009]

This issue is now at the top of my TODO list. thanks for the patience @Kynolin

One comment so far:

I haven't quite narrowed down the repeating, empty frames when pressing enter after the last command exited non-zero,

This sounds like a really one know bug/limitation. If I remember correctly, the shell integration will report a non-zero result each time you hit enter on an empty command line after a command failed. Each CR looks like a command which failed. This might be shell specific too. It's a separate (but related) issue than the main problem here.

[sedwards2009]

@Kynolin

I would imagine it would be good to have an option to select if good/bad commands always get framed.

At the moment I think it should always frame failed commands, reason being that failed commands is typically infrequent and if it does happen it is something which should get the users attention (with a big red frame).

[Kynolin]

This sounds like a really one know bug/limitation. If I remember correctly, the shell integration will report a non-zero result each time you hit enter on an empty command line after a command failed. Each CR looks like a command which failed. This might be shell specific too. It's a separate (but related) issue than the main problem here.

That's the behavior I get on both bash/fish. Pressing enter on an empty line uses the same exit code as the last command.

At the moment I think it should always frame failed commands, reason being that failed commands is typically infrequent and if it does happen it is something which should get the users attention (with a big red frame).

That's how I prefer it too, minus pressing enter on an empty line (or a line containing nothing but whitespace) and getting a bunch of empty red frames. I figured I'd at least mention having the option.

I've still been using my modified version since I apparently have a habit of pressing enter more than I realized, so no rush on my account. Besides the slightly odd behavior of when output gets converted to frames, it's been working well enough to get by without breaking tabs accidentally.

[sedwards2009]

The crashing bug is fixed in 0.47.0. Thanks. I understand the other problems better now and can attempt a solution.

[Kynolin]

Awesome, thanks! Seems to be working as expected so far. Would you prefer that I close this issue and create a new one referencing the unexpected framing behavior I noticed? Or, is there another issue already?

Fish seems pretty good but still has issues framing after the clear command, until output has filled the screen. Bash is a little worse still.

This gets framed in bash.

echo -e "1\n2\n3\n4\n5"

This does not get framed. Each line also has a very small delay in being displayed, so you can actually see each individual line be rendered one at a time. I was using frame output if longer than 3 lines. If I change it back to 1, only the line containing 5 gets framed.

for i in $(seq 1 5); do echo $i; done

[sedwards2009]

@Kynolin yes, please make a new issue. It helps keep things a bit more organised and there is less chance of things falling through the cracks.

[Kynolin]

Sounds good. I've created #229 for the unexpected framing behavior.