[bug #59817] [PATCH] src/roff/troff/input.cpp: Flatten .ec parser

[Dave]

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/