Two versus four characters

Bruce Dawson

unread,

Feb 12, 2023, 4:36:29 PM2/12/23

to python

In crrev.com/c/4242108 I moved a Python script from chrome/updater/tools to tools/win. Due to tools/.style.yapf this means that the presubmit system and "git cl format" now think that the script should use two-space indentation instead of four.

The Chromium Python style guide says, in reference to Chromium's historical habit of using two-space indentation, that:

"New scripts should not follow these deviations, but they should be followed when making changes to files that follow them."

Should I reformat this script with two-space indentation to match the local directory, put it in a subdirectory with its own .style.yapf file that specifies four-space indentation, or reformat the other nine scripts in tools/win to four-space indentation (and put a new .style.yapf file there)?

None of these options are terrible, but I'm not loving any of them either.

--

Bruce Dawson, he/him

Mike Frysinger

unread,

Feb 12, 2023, 8:16:58 PM2/12/23

to Bruce Dawson, python

reformatting to 2 space is a regression. can you disable yapf in the file with a comment mentioning the 4 space?

--
You received this message because you are subscribed to the Google Groups "python" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python+un...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/python/CAE5mQiMwpXqvXsSLV9rRgKQUTt6%2B2YpZ7wRyqzvYbfiRFXpucg%40mail.gmail.com.

Bruce Dawson

unread,

Feb 12, 2023, 9:48:23 PM2/12/23

to Mike Frysinger, python

I depend heavily on clang format to keep my code tidy so I don't want to disable .yapf. I guess I could temporarily modify the .yapf file whenever I run clang format, and then work towards reformatting the other nine scripts.

A per-file setting would be great - I don't know if that is supported. Otherwise migrating from two to four is basically impossible.

--

Bruce Dawson, he/him

Mike Frysinger

unread,

Feb 12, 2023, 10:55:36 PM2/12/23

to Bruce Dawson, python

migrate everyone to 4 spaces (ideally using black) and call it a day

-mike

Bruce Dawson

unread,

Feb 13, 2023, 1:28:06 AM2/13/23

to Mike Frysinger, python

Yeah, that sounds best. crrev.com/c/4240735 does that. It didn't take long and is the right thing in the long term.

--

Bruce Dawson, he/him

Dirk Pranke

unread,

Feb 13, 2023, 2:57:18 PM2/13/23

to Bruce Dawson, Mike Frysinger, python

+1. I'm late to the conversation but that would've been my recommendation as well.

It is unfortunate that we have this dichotomy.

-- Dirk

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/python/CAE5mQiMMOeU5oCw00KH17nVpTZ_5F9H%3DxddrMk_MYk8%3DeN8SoQ%40mail.gmail.com.

Bruce Dawson

unread,

Feb 13, 2023, 3:23:58 PM2/13/23

to Dirk Pranke, Mike Frysinger, python

It would be nice if we could opt-in on a per-file basis but I couldn't find any mention of ways to do that in the YAPF documentation. And, while it's good not to regress our indentation levels, I also find it hard to get really worked up about the need to fix it in general (as opposed to Python 2/3).

Anyway, change landed, nine scripts closer to pep-8 nirvana.

--

Bruce Dawson, he/him

Dirk Pranke

unread,

Feb 13, 2023, 4:28:13 PM2/13/23

to Bruce Dawson, Mike Frysinger, python

Yeah, I would probably only update files very opportunistically. Though I also wouldn't turn down CLs that changed things to 4 space indents and just reformatted everything to be yapf-happy.

(Though I would probably spend some time comparing the output of yapf and black. I'm not sure how much the former is maintained these days).

-- Dirk

Reply all

Reply to author

Forward