Bug 217669

Summary: Wrong behavior for 'delete char' ansi code
Product: [Applications] konsole Reporter: Joel <joel.porquet>
Component: generalAssignee: Konsole Developer <konsole-devel>
Status: RESOLVED FIXED    
Severity: normal CC: diaxen
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: The attached patch fixes the bug.
The attached patch fixes the bug and the Q_ASSERT.

Description Joel 2009-12-07 01:20:49 UTC 
Version:            (using KDE 4.3.4)
OS:                Linux
Installed from:    Archlinux Packages

The character deletion ansi code "[P" fails to delete the last character of a line.

This behavior is due to the void Screen::deleteChars(int n) function in Screen.cpp. The line lenght check seems wrong:

208	    if ( cuX+n >= screenLines[cuY].count() ) 
209	        n = screenLines[cuY].count() - 1 - cuX;

should probably be:

208	    if ( cuX+n > screenLines[cuY].count() ) 
209	        n = screenLines[cuY].count() - cuX;

which would be coherent with the existing Q_ASSERT:

212	    Q_ASSERT( cuX+n < screenLines[cuY].count() );

Regards,
Comment 1 Alexandre Becoulet 2009-12-14 14:11:45 UTC 
*** This bug has been confirmed by popular vote. ***
Comment 2 Alexandre Becoulet 2010-04-20 05:13:26 UTC 
The following bash command trigger the bug:

echo -n $'AB\x1b[1D\x1b[1P' ; sleep 5

This command writes 'AB' then moves backward with ESC [1D and fails to delete the 'B' character under cursor with ESC [1P. xterm, linux, screen and other terminal emulators do have the correct behavior and delete 'B'.
Comment 3 Alexandre Becoulet 2010-04-20 05:15:18 UTC 
Created attachment 42911 [details]
The attached patch fixes the bug.
Comment 4 Kurt Hindenburg 2010-04-20 06:15:37 UTC 
Did that patch actually work for you?  When I did, the ASSERT crashed my konsole.
Comment 5 Alexandre Becoulet 2010-04-20 15:04:58 UTC 
Yes, I used it for a while on kdebase-4.3.4 before upgrading recently. I might not have Q_ASSERT enabled.

I realize that this line seems wrong too:

    Q_ASSERT( cuX+n < screenLines[cuY].count() )

    screenLines[cuY].remove(cuX,n);

For instance lets assume curX == 0, Q_ASSERT prevents removing all characters on the line. However screenLines[cuY].remove(0, screenLines[cuY].count()) is a valid operation. Isn't it ?
Comment 6 Alexandre Becoulet 2010-04-23 02:08:04 UTC 
Created attachment 42977 [details]
The attached patch fixes the bug and the Q_ASSERT.
Comment 7 Kurt Hindenburg 2010-04-27 17:22:32 UTC 
SVN commit 1119662 by hindenburg:

Correct ANSI's 'delete char' to delete the last character in a line.

Patch provided by Alexandre Becoulet

BUG: 217669


 M  +3 -3      Screen.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1119662
Comment 8 Kurt Hindenburg 2010-04-28 04:41:27 UTC 
SVN commit 1119960 by hindenburg:

Correct ANSI's 'delete char' to delete the last character in a line.

Patch provided by Alexandre Becoulet

CCBUG: 217669


 M  +3 -3      Screen.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1119960