Bug 90201 - copy / paste removes whitespace if it is the last character of wrapping line
Summary: copy / paste removes whitespace if it is the last character of wrapping line
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Unclassified
Component: general (show other bugs)
Version: CVS
Platform: Compiled Sources Linux
: NOR normal with 60 votes (vote)
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
: 96398 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-09-25 00:34 UTC by Ralf Hemmenstaedt
Modified: 2009-04-20 02:11 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch to preserve trailing spaces when copying wrapped lines (477 bytes, patch)
2004-09-26 03:05 UTC, Ralf Hemmenstaedt
Details
Delete code removing spaces at end of lines (644 bytes, patch)
2009-03-14 04:51 UTC, Kurt Hindenburg
Details
Don't remove spaces at end of lines that wrap (964 bytes, patch)
2009-03-19 17:01 UTC, Kurt Hindenburg
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Hemmenstaedt 2004-09-25 00:34:15 UTC
Version:            (using KDE KDE 3.3.0)
Installed from:    SuSE RPMs
OS:                Linux

When copying a long wrapping line from a bash prompt and the last character is a whitespace then this whitespace gets removed.
In other words: when the konsole width is set to 80 and the character on column 80 is a whitespace it does not get copied.

How to reproduce:
Example on the bash prompt:

                                |COLUMN 80
# ... commands ... rm /tmp/file1 [eol]
/tmp/file2 ...

When doing a copy and paste into another terminal the result is:

# ... commands ... rm /tmp/file1/[eol]
tmp/file2 ...

So "rm /tmp/file1 /tmp/file2" becomes "rm /tmp/file1/tmp/file2" (and will fail).

This behaviour can be reproduced with any column width and with any destination application (konsole, kwrite, ...).

Ralf
Comment 1 Ralf Hemmenstaedt 2004-09-25 21:13:58 UTC
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).
Comment 2 Ralf Hemmenstaedt 2004-09-26 03:05:33 UTC
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
Comment 3 Kurt Hindenburg 2004-12-17 08:06:39 UTC
This bug is still in CVS/HEAD;  I'll test Ralf's patch and see if I can sneak it in for KDE3.4
Comment 4 Kurt Hindenburg 2004-12-17 17:15:27 UTC
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.
Comment 5 Kurt Hindenburg 2004-12-18 07:14:26 UTC
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)


Comment 6 Kurt Hindenburg 2005-01-06 08:00:48 UTC
*** Bug 96398 has been marked as a duplicate of this bug. ***
Comment 7 Jeff Mitchell 2008-07-31 22:35:37 UTC
This bug is back in current SVN.  I can reproduce it consistently.
Comment 8 Jeff Mitchell 2008-09-25 14:53:57 UTC
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...
Comment 9 Shlomi Fish 2008-10-09 23:34:04 UTC
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
Comment 10 Jeff Mitchell 2008-10-09 23:47:57 UTC
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...
Comment 11 Vedran Sego 2009-02-21 20:04:26 UTC
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).
Comment 12 Kurt Hindenburg 2009-03-14 04:51:34 UTC
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
Comment 13 Kurt Hindenburg 2009-03-19 04:06:20 UTC
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
Comment 14 Kurt Hindenburg 2009-03-19 04:11:49 UTC
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
Comment 15 Michael Pyne 2009-03-19 04:14:57 UTC
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.
Comment 16 Kurt Hindenburg 2009-03-19 15:30:02 UTC
Yes I agree, thanks for the info.  The fix for this bug needs to be done another way.
Comment 17 Kurt Hindenburg 2009-03-19 15:38:19 UTC
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
Comment 18 Kurt Hindenburg 2009-03-19 17:01:25 UTC
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.
Comment 19 Kurt Hindenburg 2009-03-25 05:49:30 UTC
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
Comment 20 Kurt Hindenburg 2009-04-20 02:11:48 UTC
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