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

Issue 737568 link

Starred by 2 users
Status: Untriaged
Owner: ----
Cc:
█ nick@chromium.org
dcheng@chromium.org
primiano@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

`git cl format` should not reformat files that have merely been moved

Project Member Reported by thakis@chromium.org, Jun 28 2017 
Context https://groups.google.com/a/chromium.org/forum/#!topic/cxx/iXO72ePN4Jk -- sounds like `git cl format` formats files that have been `git mv`d.

It shouldn't do that, it only should reformat lines that have "really" been touched.

We run `git diff -U0` (here: https://cs.chromium.org/chromium/tools/depot_tools/git_cl.py?type=cs&q=clang-format-diff.py+package:%5Echromium$&l=5846)

Do we just need to tack on `-M90%` or some such to that?

cc'ing a collection of clang-format and git folks.

Comment 1 by primiano@chromium.org, Jun 28 2017 

I think that -M90% -C should be reasonable enough. For some reasons I don't fully understand you also need -C (aka --find-copies).
Also, mind that the relative order of -M90% and -C matters! (see below)

I just did test this on a recent CL I reviewed where gerrit was dumb enough to not realize about the move:
https://chromium-review.googlesource.com/c/545675/

If you 
git fetch https://chromium.googlesource.com/chromium/src refs/changes/75/545675/7

and then play with the diff of FETCH_HEAD^..FETCH_HEAD

by default:

$ git diff FETCH_HEAD^..FETCH_HEAD --stat
 components/tracing/BUILD.gn                                                                                                                     |   7 +-
 components/tracing/common/DEPS                                                                                                                  |   7 +
 components/tracing/common/process_metrics_memory_dump_provider.cc                                                                               | 759 ++-----------------------------------------------------------------------------------
 components/tracing/common/process_metrics_memory_dump_provider.h                                                                                |  70 +-------
 services/resource_coordinator/BUILD.gn                                                                                                          |   5 +
 services/resource_coordinator/public/cpp/BUILD.gn                                                                                               |   2 +
 services/resource_coordinator/public/cpp/memory_instrumentation/process_metrics_memory_dump_provider.cc                                         | 771 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 services/resource_coordinator/public/cpp/memory_instrumentation/process_metrics_memory_dump_provider.h                                          |  95 +++++++++++
 {components/tracing/common => services/resource_coordinator/public/cpp/memory_instrumentation}/process_metrics_memory_dump_provider_unittest.cc |   6 +-
 9 files changed, 898 insertions(+), 824 deletions(-)


If you just use -M90% still the same:

$ git diff FETCH_HEAD^..FETCH_HEAD --stat -M'90%'
 components/tracing/BUILD.gn                                                                                                                     |   7 +-
 components/tracing/common/DEPS                                                                                                                  |   7 +
 components/tracing/common/process_metrics_memory_dump_provider.cc                                                                               | 759 ++-----------------------------------------------------------------------------------
 components/tracing/common/process_metrics_memory_dump_provider.h                                                                                |  70 +-------
 services/resource_coordinator/BUILD.gn                                                                                                          |   5 +
 services/resource_coordinator/public/cpp/BUILD.gn                                                                                               |   2 +
 services/resource_coordinator/public/cpp/memory_instrumentation/process_metrics_memory_dump_provider.cc                                         | 771 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 services/resource_coordinator/public/cpp/memory_instrumentation/process_metrics_memory_dump_provider.h                                          |  95 +++++++++++
 {components/tracing/common => services/resource_coordinator/public/cpp/memory_instrumentation}/process_metrics_memory_dump_provider_unittest.cc |   6 +-
 9 files changed, 898 insertions(+), 824 deletions(-)

And if you do 

$ git diff FETCH_HEAD^..FETCH_HEAD --stat -C -M'90%'
 components/tracing/BUILD.gn                                                                                                                     |   7 +-
 components/tracing/common/DEPS                                                                                                                  |   7 +
 components/tracing/common/process_metrics_memory_dump_provider.cc                                                                               | 759 ++-----------------------------------------------------------------------------------
 components/tracing/common/process_metrics_memory_dump_provider.h                                                                                |  70 +-------
 services/resource_coordinator/BUILD.gn                                                                                                          |   5 +
 services/resource_coordinator/public/cpp/BUILD.gn                                                                                               |   2 +
 services/resource_coordinator/public/cpp/memory_instrumentation/process_metrics_memory_dump_provider.cc                                         | 771 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 services/resource_coordinator/public/cpp/memory_instrumentation/process_metrics_memory_dump_provider.h                                          |  95 +++++++++++
 {components/tracing/common => services/resource_coordinator/public/cpp/memory_instrumentation}/process_metrics_memory_dump_provider_unittest.cc |   6 +-
 9 files changed, 898 insertions(+), 824 deletions(-)

still not enough.


But finally

$ git diff FETCH_HEAD^..FETCH_HEAD --stat -M'90%' -C
 components/tracing/BUILD.gn                                                                                                                     |   7 +-
 components/tracing/common/DEPS                                                                                                                  |   7 +
 components/tracing/common/process_metrics_memory_dump_provider.cc                                                                               | 759 ++-------------------------------------------------------------------------------------
 components/tracing/common/process_metrics_memory_dump_provider.h                                                                                |  70 +-------
 services/resource_coordinator/BUILD.gn                                                                                                          |   5 +
 services/resource_coordinator/public/cpp/BUILD.gn                                                                                               |   2 +
 {components/tracing/common => services/resource_coordinator/public/cpp/memory_instrumentation}/process_metrics_memory_dump_provider.cc          |  31 ++--
 {components/tracing/common => services/resource_coordinator/public/cpp/memory_instrumentation}/process_metrics_memory_dump_provider.h           |  18 +--
 {components/tracing/common => services/resource_coordinator/public/cpp/memory_instrumentation}/process_metrics_memory_dump_provider_unittest.cc |   6 +-
 9 files changed, 57 insertions(+), 848 deletions(-)

Does what you want. Lovely no?

Comment 2 by dtapu...@chromium.org, Oct 26 2017

Components: Tools
►

Sign in to add a comment