New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 899797 link

Starred by 1 user
Status: Untriaged
Owner: ----
Cc:
agrieve@chromium.org
█ abenner@google.com
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

`git cl format --python` is bad at formatting list slices

Project Member Reported by thakis@chromium.org, Oct 29 
I tried `git cl format --python` on https://chromium-review.googlesource.com/c/chromium/src/+/1303887/10/build/toolchain/win/ml.py

It mostly looks reasonable, but it seems to do objectively worse with slice syntax:

-  del objdata[coff_header.PointerToSymbolTable + debug_sym * SYM.size():
-              coff_header.PointerToSymbolTable + (debug_sym + 2) * SYM.size()]
+  del objdata[coff_header.PointerToSymbolTable +
+              debug_sym * SYM.size():coff_header.PointerToSymbolTable +
+              (debug_sym + 2) * SYM.size()]




-  del objdata[
-      COFFHEADER.size() + debug_section_index * SECTIONHEADER.size():
-      COFFHEADER.size() + (debug_section_index + 1) * SECTIONHEADER.size()]
+  del objdata[COFFHEADER.size() +
+              debug_section_index * SECTIONHEADER.size():COFFHEADER.size() +
+              (debug_section_index + 1) * SECTIONHEADER.size()]



(Unrelatedly, this is also harder to read:
-  SECTIONHEADER = Struct('SECTIONHEADER',
-                         '8s', 'Name',
-                         'I', 'VirtualSize',
-                         'I', 'VirtualAddress',
-
-                         'I', 'SizeOfRawData',
-                         'I', 'PointerToRawData',
-                         'I', 'PointerToRelocations',
-                         'I', 'PointerToLineNumbers',
-
-                         'H', 'NumberOfRelocations',
-                         'H', 'NumberOfLineNumbers',
-                         'I', 'Characteristics')
+  SECTIONHEADER = Struct('SECTIONHEADER', '8s', 'Name', 'I', 'VirtualSize', 'I',
+                         'VirtualAddress', 'I', 'SizeOfRawData', 'I',
+                         'PointerToRawData', 'I', 'PointerToRelocations', 'I',
+                         'PointerToLineNumbers', 'H', 'NumberOfRelocations',
+                         'H', 'NumberOfLineNumbers', 'I', 'Characteristics')


But I don't have a great suggestion for what to do about that.)


(I didn't see a reply to my "how do I report issues" question, so filing a crbug with a pyformat label for now.)

Comment 1 by abenner@google.com, Oct 29 

Yes the slice formatting looks pretty bad.
https://github.com/google/yapf this is the formatter we're using currently.
I'll try and play around with configs and see if anything helps. Otherwise I will file an issue there.

I think the struct would probably be a good case where we'd like to turn yapf formatting off, since this case was very deliberate hand formatting to make it more readable. You can turn formatting off by wrapping it with #yapf disable, #yapf enable. But obviously that's harder to do if we're trying to do a larger mechanical refactor.
►

Sign in to add a comment