Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
-+-
 Reporter:  ewong|  Owner:  (none)
 Type:  enhancement  | Status:  needs_revision
 Priority:  Low  |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Trivial  | Resolution:
 Keywords:  code-style, review-group-24  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * priority:  Medium => Low
 * milestone:  Tor: 0.3.3.x-final => Tor: unspecified


Comment:

 I don't feel too strongly that the comma usage detected here actually _is_
 bad style, so since the patch isn't working, I'm throwing it back into
 unspecified.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
-+-
 Reporter:  ewong|  Owner:  (none)
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Trivial  | Resolution:
 Keywords:  code-style, review-group-24  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by nickm):

 I've played with this some more, and run into trouble: The patch not only
 detects ,s inside of code, but also commas inside of comments and certain
 multiline strings.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
-+-
 Reporter:  ewong|  Owner:  (none)
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Trivial  | Resolution:
 Keywords:  code-style, review-group-24  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by ewong):

 Replying to [comment:18 nickm]:
 > Hm. Did you use a script to add the missing spaces, or did you do it by
 hand?

 I did it by hand.

 >
 > Ideally, there should be:
 >   * one commit that changes checkSpace.pl and nothing else.
 >   * a script that we can run on master to add the missing spaces when we
 merge.  This way, we don't get any conflicts, and it's easy to verify that
 the script does only what it claims.  Maybe something like this?
 > {{{
 > perl -i -pe 's/,(\S)/, $1/g' src/{common,or,ext,test,tools}/*.[ch]
 > }}}
 >   * And then we can just run wrap the lines by hand.
 >
 > I've extracted the script as its own commit in a branch
 `ticket23500_033` in my public repository at
 https://gitweb.torproject.org/nickm/tor.git .  If it looks okay, and
 somebody confirms the steps above, I can merge it and do the rest.
 >
 > Also, is there a reason to have the checkSpaces script check for
 `[\w|\d]+` instead of just `\S`?

 My regex-fu is basic at the best, so that's what I came up with.  I had a
 major
 issue with figuring out how to get sed to regex properly.

 sorry for the mess.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
-+-
 Reporter:  ewong|  Owner:  (none)
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Trivial  | Resolution:
 Keywords:  code-style, review-group-24  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * status:  needs_review => needs_revision


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
-+-
 Reporter:  ewong|  Owner:  (none)
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Trivial  | Resolution:
 Keywords:  code-style, review-group-24  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by nickm):

 Hm. Did you use a script to add the missing spaces, or did you do it by
 hand?

 Ideally, there should be:
   * one commit that changes checkSpace.pl and nothing else.
   * a script that we can run on master to add the missing spaces when we
 merge.  This way, we don't get any conflicts, and it's easy to verify that
 the script does only what it claims.  Maybe something like this?
 {{{
 perl -i -pe 's/,(\S)/, $1/g' src/{common,or,ext,test,tools}/*.[ch]
 }}}
   * And then we can just run wrap the lines by hand.

 I've extracted the script as its own commit in a branch `ticket23500_033`
 in my public repository at https://gitweb.torproject.org/nickm/tor.git .
 If it looks okay, and somebody confirms the steps above, I can merge it
 and do the rest.

 Also, is there a reason to have the checkSpaces script check for
 `[\w|\d]+` instead of just `\S`?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by ewong):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by ewong):

 thanks teor!

 there were a few bitrotted code, so I did the following:
 https://github.com/ewongbb/tor/tree/chkspace_f2

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 Thanks for your work on this.

 There's one last thing you need to fix:
 1. Put all the changes to check-spaces.pl in one commit. In the latest
 branch, the script is changed in the first and third commits.

 Once that's done, I'll send this to the release manager and see if they
 are happy with the manual mass change.
 We can use diff's ignore whitespace option to check that only whitespace
 was modified.
 But we'll still need to manually confirm that none of the whitespace was
 significant.

 Then, after final review, we need to:
 2. Merge this patch to master, and fix any spacing issues it identifies in
 new code

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by ewong):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by ewong):

 This is my attempt at a 'mass change'
 https://github.com/ewongbb/tor/tree/chkspace_f

 I had attempted to do a mass change with a script but the amount of time
 trying to
 figure out the code which should be changed compared to those that
 shouldn't
 would've been better used on reading and looking for the general instances
 that apply.  So I apologize as this 'mass change' was pretty much a manual
 process.


 Do note I took a few liberties at the following:

 1) changed indent where I think would be proper (though I probably missed
 quite
a few)
 2) reformatted some lines to make it less 'Wide'

 As such some caveats:
 a) haven't touched C code(and even then, I was just a beginner)
 b) I'm not familiar with the tor code and didn't want to do too much
 damage,
particularly with some whitespace which I think should be there but
 didn't
change.  i.e.   if conditions with all the conditions whitespaceless..
  if (i=0;i<10;i++) {... }   [no, this isn't from the code.. just
 an example]

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by teor):

 Replying to [comment:10 ewong]:
 > as it stands, the find.. command is getting difficult since I have yet
 to discover
 > a single command line which checks if the additional ','->', ' change
 will increase
 > the line length.. if so, it changes the line and then wraps the line
 near the end;
 > but that affects a bunch load of other lines.
 >
 > I'm starting to feel like just going through the list of files and
 changing
 > it manually will make it 'easier'.   might anyone have any advice?
 > thanks

 A mass replace script in one commit, then a few manual fixes in the next
 commit is fine.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by ewong):

 as it stands, the find.. command is getting difficult since I have yet to
 discover
 a single command line which checks if the additional ','->', ' change will
 increase
 the line length.. if so, it changes the line and then wraps the line near
 the end;
 but that affects a bunch load of other lines.

 I'm starting to feel like just going through the list of files and
 changing
 it manually will make it 'easier'.   might anyone have any advice?
 thanks

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by ewong):

 Replying to [comment:8 teor]:
 ve you checked that mass replace passes `make check-spaces`?
 >
 > I think this line is too long, and there may be others:
 > {{{
 > static char nil_bytes[16] = { [0]=0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0 };
 > }}}

 ah right.  Point taken.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 I'm not sure if we want to add spaces after integers in structure
 initialisations, or in MOCK_IMPL declarations. But it seems ok to me. I'll
 leave it to nickm to make the final call.

 Also, have you checked that mass replace passes `make check-spaces`?

 I think this line is too long, and there may be others:
 {{{
 static char nil_bytes[16] = { [0]=0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0 };
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by ewong):

 please note..  due to confusion with rebasing and merging,  I decided to
 create a new PR.

 https://github.com/ewongbb/tor/tree/chkspace

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by teor):

 Replying to [comment:5 ewong]:
 > Do you want me to do this in the same pr or a different one?

 Same pr and branch is fine, but a different commit.

 Usually we do mass replaces using a script, and then put that script in
 the commit.
 That way someone can verify that commit only contains the results of the
 script.

 For example, a script for this might be something like:
 `sed 's/,([^ ])/, \1/'`

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by ewong):

 Do you want me to do this in the same pr or a different one?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions.

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:  code-style|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * keywords:   => code-style
 * type:  defect => enhancement


Comment:

 Thanks for this patch, we'll get to it soon.

 When we have the final version of the script, it would also be helpful to
 do a mass replace patch that makes the script pass on the current
 codebase.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23500 [Core Tor/Tor]: check-spaces.pl should check spaces after a comma when in functions. (was: check-spaces.pl doesn't check for spaces not being after a comma in functions)

[Tor Bug Tracker & Wiki]

#23500: check-spaces.pl should check spaces after a comma when in functions.
--+
 Reporter:  ewong |  Owner:  (none)
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Trivial   | Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs