Follow-up Comment #2, bug #59817 (project groff):
I have no opinion on the actual code of the original vs the patch, though I
share Branden's doubt that such micro-optimizations give any measurable
benefit to execution speed.
Concerning code comments:
[comment #1 comment #1:]
> [comment #0 original submission:]
> > Comments (copied from groff(7)) are supposed to help travelers navigate
> > their way through the code base.
>
> More helpful I think would simply be a comment above the
> function giving it a specification. Individual lines of code
> should require commenting only if they're having to work
> around a bug elsewhere, or if they're doing something "clever"
I agree with Branden here: the logic is easy enough to follow that individual
branches of the conditionals don't need their own comments, but that what's
missing is an overview of what set_escape_char() does. A comment explicitly
tying this function to the user-level .ec request would be illuminating to
someone coming to this snippet cold. The logic then becomes fairly
self-explanatory.
The other patch attached here (file #50642) seems uncontroversial and worth
applying.
_______________________________________________________
Reply to this item at:
<https://savannah.gnu.org/bugs/?59817>
_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/
