From cdda9ea668cb1f7ed6479c502704ae269d2718c2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 23 Aug 2012 10:12:47 -0700 Subject: [PATCH] Bandage an issue with the file tree view for unusual versus diffs Summary: See comment inline. We should fix this properly but it goes faily deep so I just stopped the bleeding for now. It's OK if we end up with a silly-looking file tree view for bizarre edge cases. Test Plan: Created a problem diff as described, verified it no longer threw. Reviewers: vrana, btrahan Reviewed By: vrana CC: aran, Avish Maniphest Tasks: T1702 Differential Revision: https://secure.phabricator.com/D3373 --- .../DifferentialRevisionViewController.php | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 6e8e16ed5f..9d2dfebfd9 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1021,7 +1021,31 @@ final class DifferentialRevisionViewController extends DifferentialController { $tree = new PhutilFileTree(); foreach ($changesets as $changeset) { - $tree->addPath($changeset->getFilename(), $changeset); + try { + $tree->addPath($changeset->getFilename(), $changeset); + } catch (Exception $ex) { + // TODO: See T1702. When viewing the versus diff of diffs, we may + // have files with the same filename. For example, if you have a setup + // like this in SVN: + // + // a/ + // README + // b/ + // README + // + // ...and you run "arc diff" once from a/, and again from b/, you'll + // get two diffs with path README. However, in the versus diff view we + // will compute their absolute repository paths and detect that they + // aren't really the same file. This is correct, but causes us to + // throw when inserting them. + // + // We should probably compute the smallest unique path for each file + // and show these as "a/README" and "b/README" when diffed against + // one another. However, we get this wrong in a lot of places (the + // other TOC shows two "README" files, and we generate the same anchor + // hash for both) so I'm just stopping the bleeding until we can get + // a proper fix in place. + } } require_celerity_resource('phabricator-filetree-view-css');