891849 - mach's terminal magic causes weird jumps to the bottom

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement, P5)

Description

[Tim Taubert [:ttaubert] (inactive)]

Since recently (probably about two days ago) mach's console output always jumps to the bottom of a terminal even if the current line starts at the top. The status bar at the bottom makes a lot of sense after the whole terminal screen has been filled but not when it's actually empty (e.g. after hitting Ctrl+L). The previous behavior was a lot better, IMO.

Comment 1

[Gregory Szorc [:gps]]

This is fallout from bug 875684.

STR:

1) ctrl+l (wipe screen)
2) mach build

Output looks like:

----------------------------
|_>
|
|
|0:00.23 /usr/bin/make -f client.mk -s
| TIER: base nspr nss ...
----------------------------

Before, if the terminal was empty, new lines would be printed where the cursor is (typically at the top of the terminal) and the footer would be rendered where the cursor is. Now, the footer is always displayed at the bottom of the terminal and new lines are printed 1 line above the bottom of the terminal.

This is definitely an unintended consequence of bug 875684. I don't want to revert that patch because it is important for this terminal magic to work with screen. But, I don't think this is high enough priority for me to actively look at it.

If someone can deliver a patch that restores the old behavior and also works in screen, I will happily accept it.

Comment 2

[Sahil Chelaramani]

Attachment: Added a count to track number of lines, Seems to work on tmux

Hi, please review this patch. Seems to be working on Tmux , and the gnome terminal

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 788332 [details] [diff] [review]
Added a count to track number of lines, Seems to work on tmux

For future reference you need to add a review flag to an attachment. If you forget, it's likely patches sit around a long time until someone stumbles upon the bug.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 788332 [details] [diff] [review]
Added a count to track number of lines, Seems to work on tmux

Review of attachment 788332 [details] [diff] [review]:
-----------------------------------------------------------------

Not granting review for a few reasons.

First, global variables are mostly evil and should be avoided.

Second, this patch doesn't work on my iTerm terminal. It clears the screen at the beginning of the build and prints the footer line multiple times as the terminal window is filling up.

Comment 5

[Sahil Chelaramani]

Attachment: Patch for bug fix

Tried a different strategy to fix this bug. Please review.

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 791466 [details] [diff] [review]
Patch for bug fix

Review of attachment 791466 [details] [diff] [review]:
-----------------------------------------------------------------

This appears to work! I'll mark this checkin-needed and it will be checked (hopefully within a few hours).

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f673d2423e08

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f673d2423e08