[tor-bugs] #31759 [Core Tor/Tor]: Make "annotate_ifdef_directives" script comply with line-width limits

[Tor Bug Tracker & Wiki]

#31759: Make "annotate_ifdef_directives" script comply with line-width limits
--------------------------+------------------------------------
 Reporter:  nickm         |          Owner:  nickm
     Type:  defect        |         Status:  needs_review
 Priority:  Medium        |      Milestone:  Tor: 0.4.2.x-final
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:  042-should    |  Actual Points:  .1
Parent ID:  #31713        |         Points:
 Reviewer:  catalyst      |        Sponsor:  Sponsor31-can
--------------------------+------------------------------------

Comment (by catalyst):

 Replying to [comment:10 nickm]:
 > > I did make a comment on the pull request about how our stated (and
 enforced) limit seems to be 79 characters. Which one is correct? (I would
 say that 79 characters is better than 80, because of diff line prefixes,
 etc.)
 >
 > This is actually a subtle issue in the annotate_ifdef_directives code:
 our lines are 79 characters long, not counting the terminating newline.
 The `commented_line()` function assumes that it gets a terminating newline
 in its input, and so uses 80 as the max line width.
 >
 > But yeah, I agree this is not clear.
 >
 > I could do any of these solutions:
 >    * Document that commented_line() requires a `fmt` that ends with a
 newline, and that LINE_WIDTH includes the newline.
 >    * Change the calls to commented_line() so that they do not have a
 newline in `fmt`, and change LINE_WIDTH to 79.
 >    * ... something else?
 >
 > Is there one that you think is best?
 >

 I was going to start suggesting the second option (have `LINE_WIDTH`, etc.
 count non-newline characters. Then I checked and realized that Python
 retains newlines in strings read from files, and `writelines()` doesn't
 add line separators. So now I'm thinking we should do the first option. We
 should also note that we're assuming universal newline mode (the Python
 default, I think? I only looked up it up in Python 3.), so if the OS uses
 something like CRLF line endings, the count is still valid.

 This might be a minor thing: I just noticed that your changes produce
 unbalanced parentheses in the output (inside comments). Some of the old
 comments that it rewrote had differently-unbalanced parentheses. It would
 be nice if the script could produce balanced parentheses in the truncated
 output. I do sometimes use the balanced expression movement commands
 inside comments. Perhaps other people also do similarly? (Maybe the script
 could put the ellipsis inside the appropriate parentheses?)

 Also I agree with teor's suggestion of adding a test case for the 79 vs 80
 characters. That way we won't have to do quite so much convincing
 ourselves that the line width enforcement is correct. (Boundary condition
 tests are often good.)

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31759#comment:11>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online