Bug 90201

Additional tests show that if there are more than one whitespace at the end of line, they all get removed.

I compiled konsole 1.3.2 from kdebase 3.2.3 which behaves as expected (all whitespaces are copied). So the bug seems to be introduced with kde 3.3.0 (konsole 1.4).

Created attachment 7682 [details]
Patch to preserve trailing spaces when copying wrapped lines

I have taken a look at the source and it seems that makeString() should be
invoked with "false" as third parameter when the line to copy is wrapped. Then
the trailing spaces are copied.

With the attached patch I get the same behaviour as in konsole 1.3.2
I'm not sure if this breaks anything else.

Ralf

This bug is still in CVS/HEAD;  I'll test Ralf's patch and see if I can sneak it in for KDE3.4

The patch seems to work O.K. for me.  I see that Waldo coded that section in July.

http://webcvs.kde.org/kdebase/konsole/konsole/TEScreen.cpp?r1=1.82&r2=1.83

Unless someone objects, I'll apply the patch.

CVS commit by hindenburg: 

BUG: 90201
Fix the deleting of spaces when using copy/paste.


  M +1 -1      TEScreen.cpp   1.84


--- kdebase/konsole/konsole/TEScreen.cpp  #1.83:1.84
@@ -1265,5 +1265,5 @@ void TEScreen::getSelText(bool preserve_
 #define LINE_WRAP       do { \
                           assert(d <= columns); \
-                          *stream << makeString(m, d, true); \
+                          *stream << makeString(m, d, false); \
                           d = 0; \
                         } while(false)

*** Bug 96398 has been marked as a duplicate of this bug. ***

This bug is back in current SVN.  I can reproduce it consistently.

At Robert's request, I did some digging.  Here's what I found:

I took a look at this.  After looking at the code in Konsole::Screen::copyLineToStream(), I checked whether the 
behavior was different when copying from the history vs. the current screen 
image, and it is -- so the culprit seems to be this code, which is run only if 
copying from the current screen image.

            // ignore trailing white space at the end of the line
            for (int i = length-1; i >= 0; i--)
                if (data[i].character == ' ')
                    length--;
                else
                    break;


I'm not entirely sure what to do -- the only thing I can think of is to change 
it so that a single space character is allowed.  i.e. (with appropriate checks 
for valid array values)

if (data[i].character == ' ' && data[i-1].character == ' ')
	length--;

What do you think?  Alternately, the code could be removed entirely or changed 
to whatever happens when it goes into the history buffer -- when copying from 
the history, it says:  

            // retrieve line from history buffer.  It is assumed
            // that the history buffer does not store trailing white space
            // at the end of the line, so it does not need to be trimmed here

Obviously this isn't true as doing my copy from the history buffer does 
properly keep the spaces in...

Hi!

I can reproduce this bug here (Mandriva Linux Cooker on a Pentium 4).

(In reply to comment #8)
> At Robert's request, I did some digging.  Here's what I found:
> 
> I took a look at this.  After looking at the code in
> Konsole::Screen::copyLineToStream(), I checked whether the 
> behavior was different when copying from the history vs. the current screen 
> image, and it is -- so the culprit seems to be this code, which is run only if 
> copying from the current screen image.
> 
>             // ignore trailing white space at the end of the line
>             for (int i = length-1; i >= 0; i--)
>                 if (data[i].character == ' ')
>                     length--;
>                 else
>                     break;
> 
> 
> I'm not entirely sure what to do -- the only thing I can think of is to change 
> it so that a single space character is allowed.  i.e. (with appropriate checks 
> for valid array values)
> 
> if (data[i].character == ' ' && data[i-1].character == ' ')
>         length--;
> 
> What do you think?  Alternately, the code could be removed entirely or changed 
> to whatever happens when it goes into the history buffer -- when copying from 
> the history, it says:  
> 
>             // retrieve line from history buffer.  It is assumed
>             // that the history buffer does not store trailing white space
>             // at the end of the line, so it does not need to be trimmed here
> 
> Obviously this isn't true as doing my copy from the history buffer does 
> properly keep the spaces in...
> 

I suggest removing it entirely. It's annoying and buggy and a Do-what-I-don't-meannery.

Regards,

-- Shlomi Fish

I'm leaving this for Robert to make the call -- he said he'd look at it when he could.  I'm not quite sure what the reason is to remove last-character whitespace in the first place, but that is exactly why I'm not changing the code myself...

The only problem I can think of is what will happen in the last line, if selected with triple-click (i.e. if the whole line is selected)? Keeping additional spaces would be very bad.

This all worked nicely in the last days of KDE 3 (at least on Fedoras up to version 8), so - if possible - that part of code should be reused from there.

Btw, I test it with this bash code:
n=141; while [ $n -gt 2 ]; do echo -n "x"; let n=$n-1; done; echo "  xxxx"
(n is the with of the konsole window measured in characters).

Created attachment 32089 [details]
Delete code removing spaces at end of lines

Test this if you can.  I'll put it up on reviewboard.kde.org also

SVN commit 941200 by hindenburg:

Do not remove whitespace if it is the last character of wrapping line.

BUG: 90201


 M  +0 -8      Screen.cpp  


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

SVN commit 941201 by hindenburg:

Do not remove whitespace if it is the last character of wrapping line.

CCBUG: 90201


 M  +0 -8      Screen.cpp  


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

I will just add now that I saw this commit go in that removing trailing
whitespace is useful for e.g. copying svn diff output to a kwrite window or
something (otherwise you get a hojillion useless trailing spaces).

The real behavior that is desired is to not select past the end-of-line
character in the output, which seems to be the case now if you try
clicking-and-dragging in Konsole.

Yes I agree, thanks for the info.  The fix for this bug needs to be done another way.

SVN commit 941438 by hindenburg:

Revert previous patch; need to only delete space if at end of screen line

CCBUG: 90201


 M  +9 -0      Screen.cpp  


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

Created attachment 32262 [details]
Don't remove spaces at end of lines that wrap

Try this one.  It is against 4.2 branch but should apply to trunk.

SVN commit 944121 by hindenburg:

Do not delete whitespace if at end of wrapping line.

BUG: 90201


 M  +10 -6     Screen.cpp  


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

SVN commit 956386 by hindenburg:

Do not delete whitespace if at end of wrapping line.

CCBUG: 90201


 M  +11 -7     Screen.cpp  


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