bugGNU troff - Bugs: bug #61569, tmac/an.tmac: unnecessary...

 
 

bug #61569: tmac/an.tmac: unnecessary repeating of code

Submitted by:  Bjarni Ingi Gislason <bjarniig>
Submitted on:  Fri 26 Nov 2021 05:10:14 PM UTC
 
Category:  Macro man Severity:  2 - Minor
Item Group:  Lint Status:  Invalid
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

Fri 26 Nov 2021 10:22:44 PM UTC, comment #2: 

Dave's analysis is correct.

comment #1:

> if a then b else c
> d
> if a then e else f


It is important to note the unconditional execution of "d".  Ordering here is also critical due to the spacing and page length adjustment.

Furthermore, because "PT" is a hook macro--one that the user is invited to redefine (see groff_man(7))--it's important to me that there be only one call site for it, to aid groff users and developers in troubleshooting problems with how people handle their redefinitions.

Resolving as invalid.

G. Branden Robinson <gbranden>
Project Administrator
Fri 26 Nov 2021 08:58:55 PM UTC, comment #1: 

I haven't looked at an.tmac to see the context, but based on the snippet posted here, there is no way to avoid code repetition, and choosing which code to repeat is a judgment call.

Looking at the structure in pseudocode shows this.  This snippet amounts to:

if a then b else c
d
if a then e else f

The straightforward way to restructure this to eliminate the repetition of a results instead in repetition of d:

if a then
  b; d; e
else
  c; d; f

Which of these is "better" depends on several factors: the relative complexity of a and d currently; which of them might become more complex as the code undergoes future revisions; which part of the code is more significant to the overall flow (the current code signifies to readers that d is important, as it's part of the main flow; the rewrite says the test is the most important thing, and subsumes d inside it); and other factors.

In practice, the straightforward rewrite for this snippet...

.  ie \\n[cR] \{\
.    pl +1v
.    PT
.    pl +1v
.    sp 1v
.  \}
.  el \{\
.    sp .5i
.    PT
.    sp |1i
.  \}

...is longer by 3 lines, and not appreciably easier to read.

If you had a different rewrite in mind, please post it.

Dave <barx>
Project Member
Fri 26 Nov 2021 05:10:14 PM UTC, original submission:  

tmac/an.tmac: unnecessary repeating of code

.  ie \\n[cR] .pl +1v
.  el         .sp .5i
.  PT
.  ie \\n[cR] \{\
.    pl +1v
.    sp 1v
.  \}
.  el .sp |1i

####

  Variable 'cR' is tested twice, but once should suffice.

Bjarni Ingi Gislason <bjarniig>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by bjarniig (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-01-22 gbranden Item GroupNone => Lint
    2021-11-26 gbranden StatusNone => Invalid
        Assigned toNone => gbranden
        Open/ClosedOpen => Closed
    2021-11-26 barx Severity3 - Normal => 2 - Minor

    Back to the top


    Powered by Savane 3.9