Stefan Schimanski wrote:
Am 25.05.2007 um 08:55 schrieb Abdelrazak Younes:
But please shorten the comment a bit. It's not that tricky here
anyway.
Why shorten the comment? We should strieve for more comments not less.
Particularly in place like this where a _logical_ decision is made.
No, multiline comments are normally a sign for bad/non-logical code.
Moreover they have a natural tendency to get out of date. I am
completely for comments, especially short ones giving some higher level
idea of what is going on.
In this special case the comment isn't saying anything else than what is
visible in the if-statement directly already: "we're coming from the
front" = "front", "we are at the paragraph end" = "pos=lastpos", "we
don't check any further, we will not enter" = return false. Comments
should not repeat the code, but put it into a bigger context.
Stefan
Stefan, I guess you're right --- I like comments, so I do tend to be
wordy...
Find attached a revised patch (cursor_left_2.diff) with a one-line
comment (I don't want to get rid of it altogether).
However, I'm in favor of cursor_left_1.diff --- that solves the question
of whether or not to comment... ;) And shorter code is even better than
self-documenting code...
Dov
Index: src/Text2.cpp
===================================================================
--- src/Text2.cpp (revision 18497)
+++ src/Text2.cpp (working copy)
@@ -954,8 +954,6 @@
{
if (cur.selection())
return false;
- if (cur.pos() == cur.lastpos())
- return false;
Inset * inset = front ? cur.nextInset() : cur.prevInset();
if (!isHighlyEditableInset(inset))
return false;
Index: src/Text2.cpp
===================================================================
--- src/Text2.cpp (revision 18504)
+++ src/Text2.cpp (working copy)
@@ -954,7 +954,8 @@
{
if (cur.selection())
return false;
- if (cur.pos() == cur.lastpos())
+ // No inset to enter if we're moving forward at the end of a paragraph
+ if (cur.pos() == cur.lastpos() && front)
return false;
Inset * inset = front ? cur.nextInset() : cur.prevInset();
if (!isHighlyEditableInset(inset))
