Thread (40 messages) flat view 40 messages, 4 authors, 2016-08-15

Re: [PATCH 2/8] xdl_change_compact(): clarify code

From: Stefan Beller <hidden>
Date: 2016-08-03 22:13:42

On Wed, Aug 3, 2016 at 3:00 PM, Michael Haggerty [off-list ref] wrote:
Write things out a bit longer but less cryptically. Add more comments.
By less cryptic you mean in Git coding style ;)
The original author (do we want to cc Davido?) may disagree.
+
+                       /*
+                        * Are there any blank lines that could appear as the last
+                        * line of this group?
+                        */
IIRC this comment is not quite correct as this 'only' counts the number of
blank lines within the forward shifting section, i.e. in the movable space.

Later we use it as a boolean indicator (whether or not it is equal to 0)
to see if we can do better.

Any other change in code and comments looks good to me, but this stood out
like a sore thumb. (Probably the old heuristic as a whole stood out, but the
comment here specifically sounds /wrong/ to me in this place. How can
a question document a variable? I'd rather expect a question comment
to ease the understanding of a condition)

Thanks for reviving this topic!
Stefan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help