It would be better for the "large diff" criterion to be based only on the files whose content changes.
Description
Description
Event Timeline
Comment Actions
I can make it a feature request. I marked it as a bug because git diff will return something that's pretty small in this scenario.
Comment Actions
I can't reproduce this:
epriestley@orbital ~/dev/phabricator $ git mv src src-moved epriestley@orbital ~/dev/phabricator $ git diff HEAD | wc -l 1084276
That is, moving src to src-moved/ in Phabricator produces a 1 million line git diff for me.
Comment Actions
I was thinking of what you see when you then do git add, git commit, and git diff HEAD..HEAD^. I might be using the wrong terminology perhaps
Comment Actions
I can't reproduce that either:
epriestley@orbital ~/dev/phabricator $ git status On branch master Your branch is up-to-date with 'origin/master'. nothing to commit, working directory clean epriestley@orbital ~/dev/phabricator $ for i in `seq 1 1000`; do echo $i >> file.txt; done epriestley@orbital ~/dev/phabricator $ git add file.txt epriestley@orbital ~/dev/phabricator $ git commit -m 'new file' [master d968847] new file 1 file changed, 1000 insertions(+) create mode 100644 file.txt epriestley@orbital ~/dev/phabricator $ git diff HEAD..HEAD^ diff --git a/file.txt b/file.txt deleted file mode 100644 index 1179824..0000000 --- a/file.txt +++ /dev/null @@ -1,1000 +0,0 @@ -1 -2 -3 -4 -5 -6 -7 -8 -9 -10 -11 -12 -13 ... <snip a thousand lines> ... -980 -981 -982 -983 -984 -985 -986 -987 -988 -989 -990 -991 -992 -993 -994 -995 -996 -997 -998 -999 -1000 epriestley@orbital ~/dev/phabricator $
Comment Actions
Yes, I'm not claiming that git can somehow not show actual changes, just that it doesn't show file renames. This is what I was doing:
$ du -ch app | tail -n1 5.5M total $ git mv app app-moved $ git commit -m "WIP" -n $ git diff HEAD^..HEAD | wc -l 3780
Comment Actions
I can't reproduce that:
epriestley@orbital ~/dev/phabricator $ git mv src src-moved
epriestley@orbital ~/dev/phabricator $ git commit -m "WIP" -n
[master 66b4baf] WIP
git 4636 files changed, 0 insertions(+), 0 deletions(-)
rename {src => src-moved}/__phutil_library_init__.php (100%)
rename {src => src-moved}/__phutil_library_map__.php (100%)
rename {src => src-moved}/__tests__/PhabricatorCelerityTestCase.php (100%)
rename {src => src-moved}/__tests__/PhabricatorConduitTestCase.php (100%)
rename {src => src-moved}/__tests__/PhabricatorInfrastructureTestCase.php (100%)
rename {src => src-moved}/__tests__/PhabricatorLibraryTestCase.php (100%)
...<snip 4000 lines>...
rename {src => src-moved}/view/phui/calendar/PHUICalendarListView.php (100%)
rename {src => src-moved}/view/phui/calendar/PHUICalendarMonthView.php (100%)
rename {src => src-moved}/view/phui/calendar/PHUICalendarWeekView.php (100%)
rename {src => src-moved}/view/phui/calendar/PHUICalendarWidgetView.php (100%)
rename {src => src-moved}/view/viewutils.php (100%)
rename {src => src-moved}/view/widget/AphrontKeyboardShortcutsAvailableView.php (100%)
rename {src => src-moved}/view/widget/AphrontStackTraceView.php (100%)
rename {src => src-moved}/view/widget/bars/AphrontBarView.php (100%)
rename {src => src-moved}/view/widget/bars/AphrontGlyphBarView.php (100%)
rename {src => src-moved}/view/widget/bars/AphrontProgressBarView.php (100%)
epriestley@orbital ~/dev/phabricator $ git diff HEAD^..HEAD | wc -l
1084276Comment Actions
Wow, that's strange. Is it possible that you have a different setting for diff.renames?
$ git config -l | grep diff.renames diff.renames=false $ git diff HEAD^..HEAD | wc -l 196072
Comment Actions
The behavior appears to have changed in Git 2.9.0, which was released in mid June of this year:
https://github.com/git/git/blob/v2.9.0/Documentation/RelNotes/2.9.0.txt#L7
Is the new behavior to completely hide renamed files from the output?
Comment Actions
$ git diff HEAD^..HEAD | head diff --git a/app/client/config/BaseStyles.js b/app-moved/client/config/BaseStyles.js similarity index 100% rename from app/client/config/BaseStyles.js rename to app-moved/client/config/BaseStyles.js diff --git a/app/client/config/DateTimeFormats.json b/app-moved/client/config/DateTimeFormats.json similarity index 100% rename from app/client/config/DateTimeFormats.json rename to app-moved/client/config/DateTimeFormats.json diff --git a/app/client/config/categories.json b/app-moved/client/config/categories.json similarity index 100%
Comment Actions
I suspect that diff.renames has the same behavior from recent versions, just that it is enabled by default.
Comment Actions
Okay. If I diff this:
$ git -c diff.renames=true diff HEAD^ diff --git a/src/infrastructure/sms/worker/PhabricatorSMSDemultiplexWorker.php b/src/infrastructure/sms/nonworker/PhabricatorSMSDemultiplexWorker.php similarity index 100% rename from src/infrastructure/sms/worker/PhabricatorSMSDemultiplexWorker.php rename to src/infrastructure/sms/nonworker/PhabricatorSMSDemultiplexWorker.php diff --git a/src/infrastructure/sms/worker/PhabricatorSMSSendWorker.php b/src/infrastructure/sms/nonworker/PhabricatorSMSSendWorker.php similarity index 100% rename from src/infrastructure/sms/worker/PhabricatorSMSSendWorker.php rename to src/infrastructure/sms/nonworker/PhabricatorSMSSendWorker.php diff --git a/src/infrastructure/sms/worker/PhabricatorSMSWorker.php b/src/infrastructure/sms/nonworker/PhabricatorSMSWorker.php similarity index 100% rename from src/infrastructure/sms/worker/PhabricatorSMSWorker.php rename to src/infrastructure/sms/nonworker/PhabricatorSMSWorker.php
...into Phabricator, I get this:
https://secure.phabricator.com/differential/diff/39679/
Here's what I see, exactly:
Although we list both source and destination files, that looks very similar to what Git is outputting. What are you expecting Phabricator to display instead?
Comment Actions
If I make a trivial change to one, I get this:
https://secure.phabricator.com/differential/diff/39680/
In an image:
This generally looks similar to the output of git to me.
Comment Actions
I wonder why I had a different behavior:
Total actual changed lines was only like 30 lines
Comment Actions
What are you expecting to see there instead, exactly?
If you move 999,997 files and change 3, do you want all 1,000,000 paths to be visible on the page when you load it without any additional clicking?
Comment Actions
I was expecting to see exactly the view in your screenshot -- changed files expanded inline, and moved files described as such.
In my revision, all files were collapsed (renames and changes). Could this be because I didn't git mv in that case, I moved the file and then git added?
Comment Actions
It's because more than 100 total paths were affected by any kind of edit.
You will see my screenshot if your change affects between 1 and 99 paths. You will see your screenshot if it affects more than 100 paths. I don't remember which bucket exactly 100 files end up in. (In Diffusion, there is additional behavior for changes which affect more than 1,000 paths).
It's not clear to me what your desired behavior is or why the 100-file behavior is a problem.
Comment Actions
(Or how our behavior differs materially from the behavior of git diff, except that you have to click one extra time on "Show All Files Inline".)
Comment Actions
Got it.
I was actually afraid to click 'Show all files inline" because I expected it to load all the content, even the moves. I agree that it's not a huge deal to have to click this, now that I know it isn't going to do that.
The behavior I was originally asking for was exactly the behavior you see after clicking that button.
I was surprised that the "diff is very large" criterion is also deliberately based on the # of files with any kind of edit: I was assuming that it would be only based on the size of the diff that needs to be rendered on the page (which is why I thought that we were inferring the size of the diff incorrectly). I don't see what we gain by basing it on the # of files, since we render all the file names anyway.
Comment Actions
So the button already does what you want, but instead of clicking it you filed a bug report?