Add pyformat to "git cl format" by default |
|||||
Issue descriptionAuto-format python code. https://github.com/myint/pyformat https://github.com/google/yapf Once we have auto-formatting, we could disable the formatting-related checks from pylint. Experimental support for this via yapf was added in 2015: https://codereview.chromium.org/1156743008 The relevant code is here: https://cs.chromium.org/chromium/tools/depot_tools/git_cl.py?rcl=8bdc1b8a0433e3e0737bab3a31bd6fba39383610&l=5952 You can try it out by: 1) Make a change to any python file 2) Copy .style.yapf from depot_tools to src 3) Run git cl format --python --full What concrete work needs to be done here: * Make it use depot_tool's .style.yapf when none exists in the current checkout * Add support for non --full mode (bug in comments looks to be fixed... probably just need to update version of yapf) * Enable in git cl format by default (without --python)
,
Sep 24
,
Sep 24
,
Sep 28
,
Oct 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/c08566e6e27cf671133e2787fd49ca9451535163 commit c08566e6e27cf671133e2787fd49ca9451535163 Author: Aiden Benner <abenner@google.com> Date: Wed Oct 03 17:52:42 2018 Add non --full support to python git cl format Parses git diff for changed python files in a method similar to clang-diff and feeds the resulting line ranges into yapf. Also sets the default style to the yapf file included in depot tools by searching parent directories of each changed file to find a yapf style config (.style.yapf). If none is found the default style file will be the chromium .style.yapf included in depot tools. Note: Even if line ranges are specified, yapf will fix indentation issues for the entire file. This is intended see https://github.com/google/yapf/issues/499 This may cause some issues if git cl format is run on a file with lots of indentation issues or on a file or when run on a third_party file that is formatted with pep8 and does not include a .style.yapf and may make many more changes then the user expects. Still undecided on whether this should be turned on by default but if not I think the non --full support is a positive change anyways. Bug:846432 Change-Id: Ib85797f4a8e1021870901ff465ec10f7e70deb87 Reviewed-on: https://chromium-review.googlesource.com/1249642 Commit-Queue: Aiden Benner <abenner@google.com> Reviewed-by: Nodir Turakulov <nodir@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> [modify] https://crrev.com/c08566e6e27cf671133e2787fd49ca9451535163/git_cl.py
,
Oct 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/89b09b9ca3142d7bf9262de9b4ca7948db7ab7b2 commit 89b09b9ca3142d7bf9262de9b4ca7948db7ab7b2 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Wed Oct 03 20:20:00 2018 Roll src/third_party/depot_tools 6af3aa854956..684313d6a319 (2 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/6af3aa854956..684313d6a319 git log 6af3aa854956..684313d6a319 --date=short --no-merges --format='%ad %ae %s' 2018-10-03 oprypin@webrtc.org gclient setdep doesn't need to update_depot_tools 2018-10-03 abenner@google.com Add non --full support to python git cl format Created with: gclient setdep -r src/third_party/depot_tools@684313d6a319 The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG=chromium:836566,chromium:846432 TBR=agable@chromium.org Change-Id: I7686bb5d7f4d16aa8badf940b3b9f6bc104f4ee6 Reviewed-on: https://chromium-review.googlesource.com/c/1259489 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#596342} [modify] https://crrev.com/89b09b9ca3142d7bf9262de9b4ca7948db7ab7b2/DEPS
,
Nov 1
Very cool that there's been work to add this. Seems to have a could gotchas on Windows. Running out of the box gives an error trying to execute the python script 'yapf'. I tried changing this to target 'yapf.bat' or 'python yapf' but then both seemed to have a bug:
Traceback (most recent call last):
File "C:\dev\depot_tools\yapf", line 16, in <module>
from yapf import run_main
ImportError: No module named yapf
Command "python C:\dev\depot_tools\yapf --style C:\dev\depot_tools\.style.yapf scripts/run_code_generation.py -l 22-23 -l 27-27 -l 32-32 -l 35-36 -l 39-45 -l 165-169 -l 172-178 -l 187-189 -l 191-192 -l 194-195 -l 197-199 -l 202-215 -i" failed.
Could be a slight bug with how the parameters are escaped with quotes somehow?
,
Nov 7
Better summary of remaining tasks (Comment 8 was previous draft of this): 1) Get this working on a Windows checkout 2) Add a //docs/yapf.md similar to clang_format.md, and link to it from //styleguide/styleguide.md 3) Add //build/.style.yapf so that this can be tested for a while for //build 4) Add //third_party/blink/tools/blinkpy/.style.yapf so that it stays as pep8 5) Add a root .style.yapf to enable for chromium.
,
Nov 7
Will enabling it by default be after 5)?
,
Nov 7
I think the plan is to have py files that have a .style.yapf in an ancestor directory to be formatted by default. The --python flag to run over all py files (and use the chromium default if no style file is found)
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/e47ac15d93b88d76e38e56054a15dfb916b30112 commit e47ac15d93b88d76e38e56054a15dfb916b30112 Author: Aiden Benner <abenner@google.com> Date: Tue Nov 20 16:44:03 2018 Fix git cl format --python on windows git cl format --python currently breaks on windows because FindExecutable('yapf') returns .../depot_tools/yapf (a py file) instead of .../depot_tools/yapf.bat. Also yapf.bat tries to run the yapf py file without vpython which breaks the yapf dependency. This CL fixes these two issues. Bug:846432 Change-Id: I551a4c1e6367074fa76767851bd34feb2dcfb6a2 Reviewed-on: https://chromium-review.googlesource.com/c/1341236 Reviewed-by: Robbie Iannucci <iannucci@chromium.org> Commit-Queue: Aiden Benner <abenner@google.com> [modify] https://crrev.com/e47ac15d93b88d76e38e56054a15dfb916b30112/yapf.bat [modify] https://crrev.com/e47ac15d93b88d76e38e56054a15dfb916b30112/git_cl.py
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42999ec41a129ac385fbd8007e883bce9ae4f0f7 commit 42999ec41a129ac385fbd8007e883bce9ae4f0f7 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Tue Nov 20 19:48:34 2018 Roll src/third_party/depot_tools 7da982abf99f..e47ac15d93b8 (1 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/7da982abf99f..e47ac15d93b8 git log 7da982abf99f..e47ac15d93b8 --date=short --no-merges --format='%ad %ae %s' 2018-11-20 abenner@google.com Fix git cl format --python on windows Created with: gclient setdep -r src/third_party/depot_tools@e47ac15d93b8 The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG=chromium:846432 TBR=agable@chromium.org Change-Id: I2915800b137f3679f918b5cfa488a70469505ffe Reviewed-on: https://chromium-review.googlesource.com/c/1344271 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#609777} [modify] https://crrev.com/42999ec41a129ac385fbd8007e883bce9ae4f0f7/DEPS
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/99b0ccb45a0f5b976040c93811818ecdbe83471f commit 99b0ccb45a0f5b976040c93811818ecdbe83471f Author: Aiden Benner <abenner@google.com> Date: Tue Nov 20 19:53:31 2018 Enable python formatting by default if yapf file provided Enables python formatting in git cl format by default for all files that have a .style.yapf file in a parent directory up to the repository root. If no .style.yapf file is found the --python flag still needs to be set explicitly for python formatting to be enabled. Also adds a --no-python to explicitly disable python formatting on any file even if a .style.yapf file is found. This allows default formatting to be enabled on a per project/directory basis by adding a .style.yapf file. Bug:846432 Change-Id: I40d899ec1a3e0dfca445e04b91befab113416175 Reviewed-on: https://chromium-review.googlesource.com/c/1316415 Commit-Queue: Aiden Benner <abenner@google.com> Reviewed-by: Nodir Turakulov <nodir@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> [modify] https://crrev.com/99b0ccb45a0f5b976040c93811818ecdbe83471f/presubmit_canned_checks.py [modify] https://crrev.com/99b0ccb45a0f5b976040c93811818ecdbe83471f/git_cl.py
,
Nov 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/892936ea1d80b822d437a4590cc3bfd0ac10eb73 commit 892936ea1d80b822d437a4590cc3bfd0ac10eb73 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Wed Nov 21 09:40:22 2018 Roll src/third_party/depot_tools e47ac15d93b8..a85a4b01ee72 (9 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/e47ac15d93b8..a85a4b01ee72 git log e47ac15d93b8..a85a4b01ee72 --date=short --no-merges --format='%ad %ae %s' 2018-11-21 jbudorick@chromium.org Don't fetch flash when fetching android_internal. (reland) 2018-11-21 tandrii@chromium.org git cl: remove Rietveld implementation entirely. 2018-11-21 tandrii@chromium.org git cl: remove Rietveld import and things it depends on. 2018-11-21 tandrii@chromium.org git cl: get rid of Rietveld's upload.py import. 2018-11-20 vadimsh@chromium.org Revert "[vpython] Roll to pick up user differentiation of cached envs." 2018-11-20 iannucci@chromium.org [vpython] Roll to pick up user differentiation of cached envs. 2018-11-20 jbudorick@chromium.org Revert "Don't fetch flash when fetching android_internal." 2018-11-20 abenner@google.com Enable python formatting by default if yapf file provided 2018-11-20 jbudorick@chromium.org Don't fetch flash when fetching android_internal. Created with: gclient setdep -r src/third_party/depot_tools@a85a4b01ee72 The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG=chromium:856278,chromium:869227,chromium:869227,chromium:856278,chromium:846432,chromium:856278 TBR=agable@chromium.org Change-Id: Ifc45e8c4dda5ca7c283310fc840910250a778ffd Reviewed-on: https://chromium-review.googlesource.com/c/1345394 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#609969} [modify] https://crrev.com/892936ea1d80b822d437a4590cc3bfd0ac10eb73/DEPS
,
Nov 23
I get crashes when trying to run this in a standalone V8 checkout:
$ git cl format --python
Traceback (most recent call last):
File "/usr/local/google/home/leszeks/depot_tools/metrics.py", line 262, in print_notice_and_exit
yield
File "/usr/local/google/home/leszeks/depot_tools/git_cl.py", line 5728, in <module>
sys.exit(main(sys.argv[1:]))
File "/usr/local/google/home/leszeks/depot_tools/git_cl.py", line 5710, in main
return dispatcher.execute(OptionParser(), argv)
File "/usr/local/google/home/leszeks/depot_tools/subcommand.py", line 252, in execute
return command(parser, args[1:])
File "/usr/local/google/home/leszeks/depot_tools/metrics.py", line 247, in _inner
return self._collect_metrics(func, command_name, *args, **kwargs)
File "/usr/local/google/home/leszeks/depot_tools/metrics.py", line 200, in _collect_metrics
result = func(*args, **kwargs)
File "/usr/local/google/home/leszeks/depot_tools/git_cl.py", line 5494, in CMDformat
py_line_diffs = _ComputeDiffLineRanges(filtered_py_files, upstream_commit)
File "/usr/local/google/home/leszeks/depot_tools/git_cl.py", line 721, in _ComputeDiffLineRanges
line_diffs[curr_file].append((diff_start, diff_count))
KeyError: None
,
Nov 23
Hi leszeks. I was able to reproduce an identical crash by adding diff.noprefix to my git config. This was breaking the regex that was parsing the git diff. I will work on a fix.
,
Nov 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/6c18a1afa1a7a30421775c8d6c1cfb7edb3f272a commit 6c18a1afa1a7a30421775c8d6c1cfb7edb3f272a Author: Aiden Benner <abenner@google.com> Date: Fri Nov 23 20:18:23 2018 Fix git cl format --python when diff.noprefix flag is set Adding the diff.noprefix flag to .gitconfig broke python formatting because git diffs were being parsed with regex that relied on the default git diff prefixs. This CL fix's this issue by always specifying prefix flags for git diffs when formatting python. Bug:846432 Change-Id: Ifde305da9574e6b46455a0237703b7364fac77e4 Reviewed-on: https://chromium-review.googlesource.com/c/1349809 Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> [modify] https://crrev.com/6c18a1afa1a7a30421775c8d6c1cfb7edb3f272a/git_cl.py
,
Nov 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0cee69040efa1d80e27b0f41a55e431587e11a2 commit f0cee69040efa1d80e27b0f41a55e431587e11a2 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Fri Nov 23 21:54:09 2018 Roll src/third_party/depot_tools 25c4fce2cebb..6c18a1afa1a7 (1 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/25c4fce2cebb..6c18a1afa1a7 git log 25c4fce2cebb..6c18a1afa1a7 --date=short --no-merges --format='%ad %ae %s' 2018-11-23 abenner@google.com Fix git cl format --python when diff.noprefix flag is set Created with: gclient setdep -r src/third_party/depot_tools@6c18a1afa1a7 The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG=chromium:846432 TBR=agable@chromium.org Change-Id: I72da7b6f97e3b3026d79ae8ddddee692a11896d8 Reviewed-on: https://chromium-review.googlesource.com/c/1349444 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#610674} [modify] https://crrev.com/f0cee69040efa1d80e27b0f41a55e431587e11a2/DEPS
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a6dad4a95a96690e5f5b40460537b94e79143687 commit a6dad4a95a96690e5f5b40460537b94e79143687 Author: Andrew Grieve <agrieve@chromium.org> Date: Thu Nov 29 13:57:32 2018 python.md: Add info about YAPF and elaborate on pylint Bug: 846432 Change-Id: Iac715f99198a10409018c208588211252aa53e53 Reviewed-on: https://chromium-review.googlesource.com/c/1352819 Reviewed-by: Peter Wen <wnwen@chromium.org> Commit-Queue: Peter Wen <wnwen@chromium.org> Cr-Commit-Position: refs/heads/master@{#612178} [modify] https://crrev.com/a6dad4a95a96690e5f5b40460537b94e79143687/styleguide/python/python.md
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a1b9066b334be375ff937378b0670cb7941b92c commit 1a1b9066b334be375ff937378b0670cb7941b92c Author: agrieve <agrieve@chromium.org> Date: Fri Nov 30 15:47:22 2018 python.md: Add info about YAPF bugs and editor integrations Bug: 846432 Change-Id: Ie618f4e8f978bec4134b1b163e46f087d3101967 Reviewed-on: https://chromium-review.googlesource.com/c/1356238 Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#612658} [modify] https://crrev.com/1a1b9066b334be375ff937378b0670cb7941b92c/styleguide/python/python.md
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5caba8d74f3d888c65d18d45f3608ae2118c71fd commit 5caba8d74f3d888c65d18d45f3608ae2118c71fd Author: Andrew Grieve <agrieve@chromium.org> Date: Thu Dec 06 02:23:36 2018 Enable Python auto-formatting for //build/android This will cause presubmit to fail for deltas that don't match YAPF rules, and also enable "git cl format" to fix any such violations. Bug: 846432 Change-Id: I3602e41811474a068f4780628601a14565320db9 Reviewed-on: https://chromium-review.googlesource.com/c/1364338 Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: agrieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/master@{#614237} [add] https://crrev.com/5caba8d74f3d888c65d18d45f3608ae2118c71fd/build/android/.style.yapf
,
Jan 14
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by agrieve@chromium.org
, Sep 10