GNOME Bugzilla – Bug 506438
Ctrl-L adds blank space to the scrollback buffer
Last modified: 2021-06-10 13:54:31 UTC
Please describe the problem: Ctrl-L adds blank space to the scrollback buffer Steps to reproduce: 1. Open a vte-based terminal (observed with sakura and vte). 2. Hit <C-L> 3. Watch how the scrollbar decreases its height. 4. Hit <c-L> several times, and see that the scrollbar decreases in a non monotonical way. 5. After several <C-L>-s the scrollbar stops decreasing. Actual results: scrollbufer increases Expected results: nothing Does this happen every time? yes Other information: The shell used was bash. Compared this with mrxvt (again with bash) and it behaves "correctly".
I kinda agree that it would be more useful if ctrl+L erased the contents of the current screen instead of adding it to scrollback buffer.
Also relevant: bug 678042. I actually like that a Ctrl+L scrolls out the contents. I don't like however that subsequent Ctrl+L's add a full empty page. Maybe we could do something clever like scrolling until the screen becomes empty? Since we're clearing everything under the cursor, this would mean scrolling by as many lines as the current cursor's row. (See also bug 710806.) Sounds like a cool approach to me. (Or a CSI 2J could adjust the ring's end so that it doesn't point to the bottom row of the terminal, rather to the current cursor position (below which everything was removed). That would automatically fix a forthcoming Ctrl+L's behavior. This approach would introduce the concept of used vs. unused areas, going against bug 708213 comment 29. Sounds more complicated and fragile in the long run.)
Oh, no... me stupid. Ctrl+L (actually the "clear" terminfo capability) first moves the cursor home, then clears the screen. So the simple approach can't work. We either go for a more complicated one, or change it to work like xterm works, or just leave it as it is.
*** Bug 746625 has been marked as a duplicate of this bug. ***
In the dupe report Christoph raises a valid security concern. And now that we clear the scrollback, I think we shouldn't keep one screenful of data. I tend towards making it fully xterm-compatible. In the mean time, it'd be nice to have a bash/readline feature so that Ctrl-L manually scrolls out the contents before repainting the prompt on the top of the screen.
Created attachment 300391 [details] [review] Fix
Just for my information,... that patch would now blank the whole scrollback buffer? :)
Give it a try :) The "clear" command outputs the following three escape sequences: \e[3;J \e[H \e[2J The first one clears the whole scrollback even in current vte, this patch doesn't change it. The middle one moves the cursor home, it's irrelevant here. The last one clears the visible screen; the current patch changes its behavior not to scroll by a pageful. So, effectively, the "clear" command currently clears the scrollback except for one pageful of data (whichever was visible before issuing the "clear" command), and with this patch it totally clears it. Bash's Ctrl-L, on the other hand, only emits the 2nd and 3rd sequences, that is, it didn't and won't clear the scrollback.
Review of attachment 300391 [details] [review]: Something's wrong with this patch... Produce more than a screenful of output, then execute "clear", and then shrink the window vertically. The shell prompt scrolls out from the viewport.
The problem is probably not with this patch, but with "drop2" in vte_terminal_screen_set_size() -- below_current_paragraph.row is relative to insert_delta, so it's probably semantically incorrect to subtract this from _vte_ring_length(ring). My first attempt (to subtract insert_delta) fixes this but breaks something else. I'll investigate more thourughly a bit later.
Review of attachment 300391 [details] [review]: Reverting the patch status to "none"; there's nothing wrong with this patch, it just revealed a bug somewhere else.
Created attachment 300617 [details] [review] Fix to the underlying vertical positioning bug
I moved the underlying issue to its separate bug 747059.
Fixed in master (future 0.42).
The patch was reverted. It breaks switching to the alternate screen, see bug 747191. The new clear_screen() method was basically a copy-paste of clear_current_line() in a loop. This works for the "used" parts, but not for the "unused" area. Instead, the more complex code of handler_cd() should be copied which takes care of this too. In the mean time, I don't understand how come that the alternate screen's used vs. unused areas are the same as the normal screen's. This probably deserves some further investigation. I even see a chance for other bugs nearby that went undiscovered so far. It's yet another place where bug 747175's cleanup would help a lot to have a simpler and more robust code.
FYI: The konsole counterpart of this bug is at https://bugs.kde.org/show_bug.cgi?id=384218.
See also https://gitlab.gnome.org/GNOME/vte/issues/84 for an idea if we revert to the xterm-compatible behavior.
Good news: ncurses recently changed its logic: 20180804 + improve logic for clear with E3 extension, in case the terminal scrolls content onto its saved-lines before actually clearing the display, by clearing the saved-lines after clearing the display (report/patch by Nicholas Marriott). This means that no matter which clearing strategy we pick: xterm's which does not scroll out lines, or our current one that scrolls by a page, or a to-be-implemented smart one which takes the onscreen contents into account, the "clear" command consistently does wipe out the entire scrollback buffer. This stops the silly behavior of VTE after a Ctrl+L at the bash prompt (or the command "clear -x") that it preserved one pageful of data in the scrollback with older ncurses versions, which didn't make any sense from a user's point of view.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vte/-/issues/1471.