Bug 105373 - comment command broken when used with static word wrap
Summary: comment command broken when used with static word wrap
Status: RESOLVED WORKSFORME
Alias: None
Product: kate
Classification: Applications
Component: general (show other bugs)
Version: 2.4
Platform: Gentoo Packages Linux
: NOR normal with 101 votes (vote)
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
: 125123 139892 170881 185110 201322 240610 240790 243345 243443 248766 255300 274545 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-05-09 21:26 UTC by Xan Charbonnet
Modified: 2013-12-17 20:26 UTC (History)
18 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
patch that I believe fixes this bug? (1.19 KB, patch)
2011-06-10 14:21 UTC, Yngve Levinsen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Charbonnet 2005-05-09 21:26:08 UTC
Version:           2.4 (using KDE KDE 3.4.0)
Installed from:    Gentoo Packages
Compiler:          gcc 3.4.3 
OS:                Linux

I use CTRL-D a lot to comment blocks of Perl code.  It simply places "# " (a pound sign and a space) at the head of each highlighted line.

When I have static word wrap turned on, any lines that are within two characters of hitting the wrap limit have their last word moved to a new line (they're pushed by the two new characters).  That's all fine and dandy, but the problem is that the new line _is not commented_.  Which means the code doesn't compile and I have to go searching for the problem.
Comment 1 Anders Lund 2005-05-09 22:30:21 UTC
Monday 09 May 2005 21:26 skrev Xan Charbonnet:
> I use CTRL-D a lot to comment blocks of Perl code.  It simply places "# "
> (a pound sign and a space) at the head of each highlighted line.


Yes, as perl does not have multiline comment markers (don't suggest 'podding 
it out', it's something different ;) )

> When I have static word wrap turned on, any lines that are within two
> characters of hitting the wrap limit have their last word moved to a new
> line (they're pushed by the two new characters).  That's all fine and
> dandy, but the problem is that the new line _is not commented_.  Which
> means the code doesn't compile and I have to go searching for the problem.


Right, that is a problem.
Comment 2 Xan Charbonnet 2005-05-10 19:01:24 UTC
It would also be nice if, upon uncommenting with CTRL-SHIFT-D, the new line's contents went back to the end of the old line.  That's probably more of a wishlist thing though.
Comment 3 Anders Lund 2005-05-10 21:06:12 UTC
Tuesday 10 May 2005 19:01 skrev Xan Charbonnet:
> It would also be nice if, upon uncommenting with CTRL-SHIFT-D, the new
> line's contents went back to the end of the old line.  That's probably more
> of a wishlist thing though.


Yes :-)
Comment 4 Thomas Friedrichsmeier 2007-12-10 19:56:57 UTC
*** Bug 125123 has been marked as a duplicate of this bug. ***
Comment 5 Thomas Friedrichsmeier 2007-12-10 20:01:28 UTC
This does not only apply to perl. It's a general problem (at least for languages without multiline comments). Hence retitling.

Also confirming this for both KDE 3.5.8 and KDE 4.
Comment 6 Thomas Friedrichsmeier 2007-12-11 02:15:03 UTC
*** Bug 139892 has been marked as a duplicate of this bug. ***
Comment 7 Alessandro Rossini 2009-03-17 11:48:21 UTC
I can confirm the bug for:
Version:           3.2.1 (using KDE 4.2.1)
Installed from:    Kubuntu Intrepid 8.10

This bug is four years old now, and it is quite annoying since it regards many languages.
Any chance to see it fixed?

Best regards.
Comment 8 Dominik Haumann 2009-06-05 00:10:29 UTC
*** Bug 185110 has been marked as a duplicate of this bug. ***
Comment 9 Thomas Braun 2009-07-24 23:37:19 UTC
*** Bug 201322 has been marked as a duplicate of this bug. ***
Comment 10 tim blechmann 2010-04-03 09:58:33 UTC
i hope, it won't take another 5 years to fix this.
Comment 11 Michel Ludwig 2010-06-05 14:06:28 UTC
*** Bug 240610 has been marked as a duplicate of this bug. ***
Comment 12 Michel Ludwig 2010-06-05 14:07:19 UTC
*** Bug 240790 has been marked as a duplicate of this bug. ***
Comment 13 Michel Ludwig 2010-07-03 19:55:16 UTC
*** Bug 243345 has been marked as a duplicate of this bug. ***
Comment 14 Dominik Haumann 2010-07-04 23:23:31 UTC
*** Bug 170881 has been marked as a duplicate of this bug. ***
Comment 15 Dominik Haumann 2010-07-04 23:24:34 UTC
*** Bug 243443 has been marked as a duplicate of this bug. ***
Comment 16 Michel Ludwig 2010-08-27 17:38:01 UTC
*** Bug 248766 has been marked as a duplicate of this bug. ***
Comment 17 Yngve Levinsen 2011-04-19 18:51:13 UTC
So this bug is now 6 years old and no one is picking it up? :( 
It is annoying for me when I use Kile and static word wrap, which is my preferred setup. 
If I use dynamic word wrap then it makes a problem for svn/git and friends because you have so few lines so the chance that several authors edited the same line is high. Would really appreciate this being improved!
Comment 18 Yngve Levinsen 2011-06-10 14:21:11 UTC
Created attachment 60859 [details]
patch that I believe fixes this bug?

I managed to get annoyed enough that I had a look at the source code. Static word wrap is very practical when you keep your code in a revision control system to reduce merge conflicts, but this bug made it very unpractical for at least latex editing (using Kile).
 I wrote a small patch which at least when I tested it seemed to work. Then again, I realize that the code has dependencies right and left being a tool for so many other programs, so perhaps I with this patch break something else. Please review.
Comment 19 Dominik Haumann 2011-06-10 17:45:23 UTC
As a workaround this works.

However, if you edit the comment then at some places it will immediately break, and you still run into the problem. But still, this is better than nothing imo.
Comment 20 Dominik Haumann 2011-06-10 17:47:07 UTC
As a workaround this works.

However, if you edit the comment then at some places it will immediately break, and you still run into the problem. But still, this is better than nothing imo.
Comment 21 Yngve Levinsen 2011-06-10 19:13:41 UTC
That is true. My current gripe is when I want to comment out a paragraph (or several), and use ctrl+d on a marked set of text. It is not always that I note immediately that one or several lines went "overboard". If I am editing a comment and it does a linebreak I will notice at least (or so I believe). I believe this workaround solves the first situation.

I know it is not a perfect patch, and you as experienced developers might have a better idea. This is in fact my first patch to KDE :)
Comment 22 Dominik Haumann 2011-06-10 19:55:59 UTC
What we could do is: never wrap single line comments. This however is currently not possible, as we don't have a flag in the highlighting system that tells us whether it's a single or multiline comment.
Comment 23 Thomas Damgaard 2011-06-10 20:26:53 UTC
2011/6/10 Dominik Haumann <dhdev@gmx.de>:
> What we could do is: never wrap single line comments.

Or just break it, but add another % in the front of the next line.
Isn't this what most other editors/IDEs do? Like, say, Eclipse.

Med venlig hilsen/Kind regards
Thomas Damgaard Nielsen
http://thomasdamgaard.dk
Comment 24 Yngve Levinsen 2011-06-11 01:35:42 UTC
@Thomas I think that is a better solution, absolutely. I just didn't know how to do that since I am not yet very familiar with the source...
Comment 25 Michel Ludwig 2011-06-11 09:46:39 UTC
*** Bug 274545 has been marked as a duplicate of this bug. ***
Comment 26 Dominik Haumann 2011-06-11 10:45:03 UTC
Unfortunately it's more complex. Simply inserting e.g. a '%' for wrapped lines does not work if you edit a line as follows:

% text text text text text text text text text text text

Assume, we wrap in the next column. Insert a word as follows:

% word text text text text text text text text text text
text

Insert another words:

% word word text text text text text text text text text
text text

And so on:

% word word word word word word text text text text text
text text text text text text

In other words: Kate adds the text to be wrapped to the already wrapped text.

Now apply your solution, you'd get:

% word text text text text text text text text text text
% text

Another word:
% word word text text text text text text text text text
% text % text

And so on:
% word word word word word word text text text text text
% text % text % text % text % text % text

This is not what you want. Without the smart concatenation, you'd get:
% word word word word word word text text text text text
% text
% text
% text
% text
% text
% text

Also not what you want. This is btw the reason why it's not fixed. Because it's not that simple. Or maybe a good solution for all cases does not even exist.
Comment 27 Yngve Levinsen 2011-06-11 11:21:27 UTC
@Dominik good point, which I did not think about. I guess you'd somehow need to treat the comment character (+ whitespaces) as a separate column entity and then the wrapped text would need to start after the comment character.

I also realized another difference between my proposed patch and what Thomas suggested. If you comment out a paragraph and then uncomment it again, you are back at the same line count if you do what I propose, since you just halt the word wrap when commenting. If you break the lines when commenting you get some lines with one word.
Comment 28 Dominik Haumann 2011-06-11 13:37:40 UTC
Yes, that's why I think the patch you propose in comment #18 is a very good one as workaround.

@ comment #27 part b) Yes, another reason for just applying this patch.

We (some Kate developers) once had a discussion about static word wrap. And essentially it's probably impossible to always get it right for the user (keep the smart wrapping in mind explained in comment #26). That's the reason why it right now simply wraps, and that's it.


Christoph, ok to commit the patch?
Comment 29 Thomas Damgaard 2011-06-11 20:21:43 UTC
2011/6/11 Dominik Haumann <dhdev@gmx.de>:
> We (some Kate developers) once had a discussion about static word wrap. And
> essentially it's probably impossible to always get it right for the user (keep
> the smart wrapping in mind explained in comment #26). That's the reason why it
> right now simply wraps, and that's it.

Impossible? Really? Then, how come other editors can get this right?
Eclipse and Vim comes to mind. I am almost sure that I remember seing
Emacs doing this right also.



Med venlig hilsen/Kind regards
Thomas Damgaard Nielsen
http://thomasdamgaard.dk
Comment 30 Alvaro Aguilera 2011-06-11 20:33:05 UTC
>This is not what you want. Without the smart concatenation, you'd get:
>% word word word word word word text text text text text
>% text
>% text
>% text
>% text
>% text
>% text

that would be an acceptable solution for me. it's not perfect, but _way_ better than it currently is.
Comment 31 Yngve Levinsen 2011-06-14 19:26:53 UTC
Just to add, this is how I believe the logic of the static word wrap for a complete solution should be:
- when commenting/uncommenting lines, suspend static word wrap.
- when word wrap:
   - if current line is a comment line:
     - if next line has comment character at same indentation, push last 
       word from current line to after the comment character (+ whitespace) 
       of next line.
     - else, push last word from current line to next line, keep indendation,
       add comment character (+ whitespace) at the beginning of that line.

Such logic would be how I'd expect it to work at least (and I realize that others might be of a different opinion). I think it is perfectly possible to implement, and should not have any of the problems mentioned in comment #26.
Comment 32 Jonas 2011-07-17 11:22:53 UTC
Hi, can we expect this in the next version?

I just want to add that for me as a kile-user, it doesn't matter so much if it is the basic variant of the patch, or the more sophisticated one described in #31. It is just that I sometimes want to comment large parts of a document (like several chapters), and with the current implementation that means that I have to scroll through to check for wrapped lines each time. 

The problem of editing comments is really a comparatively minor one, so the behaviour in this case doesn't matter so much. Thus, instead of searching for a perfect solution, I suggest just including the simple one.
Comment 33 Dominik Haumann 2011-07-17 19:34:23 UTC
Git commit 30c06cf383d0fbc8ebdc125fc180dc90b4af7738 by Dominik Haumann.
Committed on 17/07/2011 at 21:32.
Pushed by dhaumann into branch 'master'.

prevent word wrap from breaking comments

CCBUG: 105373

M  +6    -0    part/document/katedocument.cpp

http://commits.kde.org/kate/30c06cf383d0fbc8ebdc125fc180dc90b4af7738
Comment 34 Dominik Haumann 2011-07-17 19:35:08 UTC
Git commit 682fa5ac9376bf5cd6e556259d531c895a31172c by Dominik Haumann.
Committed on 17/07/2011 at 21:32.
Pushed by dhaumann into branch 'KDE/4.7'.

prevent word wrap from breaking comments

CCBUG: 105373

M  +6    -0    part/document/katedocument.cpp

http://commits.kde.org/kate/682fa5ac9376bf5cd6e556259d531c895a31172c
Comment 35 Dominik Haumann 2011-07-17 19:38:29 UTC
Workaround from commment #18 committed for KDE 4.7. This works quite well. The basic problem when manually editing this comment still exists so far.

Comment #31 suggests further solution, but this also not always works. Example:
/// doxygen comment
/// some text ... doxygen list:
/// - bla bla bla bla bla bla bla
/// - foo foo foo

Wrapping bla would lead to
/// doxygen comment
/// some text ... doxygen list:
/// - bla bla bla bla bla bla bla
/// bla - foo foo foo

Hence, the list would be wrongly interpreted. In other words, since this also does not solve the issue to 100%, I'm against implementing this.
Comment 36 Yngve Levinsen 2011-07-18 07:54:10 UTC
@Dominik Ah, that one I had not thought of! I checked how static word wrap is done without comments, and then it seems to solve the issue you mention. I guess by reusing the "normal" static word wrap logic, with some added logic to treat the comments as a separate entity would work at least as well as static word wrap without comments.. Or? 

Then again I don't know enough about Kate (or programming in general) to know how difficult it would be to implement. This bug being open for now 6 years tells me it is probably not an easy matter.. :)

Happy to hear my patch made it into KDE, a small feat for me to have my first patch accepted! :)
Comment 37 Christoph Cullmann 2012-10-27 13:04:37 UTC
The workaround is ok, more logic will not went into this. Kate doesn't fully understand the text you have, therefore all other "magic" on static word wrap will just make it worse.
Comment 38 Michel Ludwig 2013-02-02 21:15:20 UTC
*** Bug 255300 has been marked as a duplicate of this bug. ***
Comment 39 Dominik Haumann 2013-12-17 20:26:27 UTC
For the record, a similar issue again appeared when removing trailing spaces on save, see bug #328900. Fixed in the same way.