Page MenuHomePhabricator
Differential D16099

Merge PhutilGitURI into PhutilURI
ClosedPublic
Actions

Authored by epriestley on Jun 13 2016, 12:41 PM.
Tags
None
Referenced Files
F14006469: D16099.id38739.diff
Mon, Oct 28, 10:33 AM
F13997764: D16099.diff
Thu, Oct 24, 6:05 AM
F13996941: D16099.id38737.diff
Thu, Oct 24, 1:13 AM
F13983380: D16099.id38735.diff
Sun, Oct 20, 6:08 AM
F13976549: D16099.diff
Fri, Oct 18, 2:45 PM
F13974498: D16099.id38733.diff
Fri, Oct 18, 5:52 AM
Unknown Object (File)
Sun, Oct 13, 6:45 AM
Unknown Object (File)
Sun, Oct 13, 6:44 AM
View All Files
Subscribers
None

Details

Reviewers
chad
Maniphest Tasks
T11137: Unable to update Git repositories with latest stable
Commits
rPHU52748950bb36: Merge PhutilGitURI into PhutilURI
rPHU8bb124c37de3: Merge PhutilGitURI into PhutilURI
Summary

Ref T11137. This addresses three general issues:

  • From T11137, I recently added a bad typehint.
  • From T11004, we currently have buggy behavior with Git URIs that have relative paths: we convert them into "ssh://" URIs but change the meaning of their paths.
  • Generally, Phabricator has a lot of instanceof-flavored code around URI handling, and this has been a frequent source of errors. See some discussion in T11137. See next revision for changes to Phabricator.
Test Plan

Unit tests; see next revision for field testing.

Diff Detail

Repository
rPHU libphutil
Branch
gituri1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 12612
Build 16013: Run Core Tests
Build 16012: arc lint + arc unit

Event Timeline

epriestley updated this revision to Diff 38733.Jun 13 2016, 12:41 PM
epriestley retitled this revision from to Merge PhutilGitURI into PhutilURI.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
epriestley added a task: T11137: Unable to update Git repositories with latest stable.
epriestley updated this revision to Diff 38734.Jun 13 2016, 12:56 PM
  • Test behavior for URIs with no protocol.
  • Test behavior for creating new Git URIs from other Git URIs.
  • Test behavior for implicit SSH protocol on Git URIs.
epriestley updated this revision to Diff 38735.Jun 13 2016, 1:10 PM
  • Remove "INVALID_PATH" faux-error.
epriestley mentioned this in D16100: Remove all uses of PhutilGitURI in Phabricator.Jun 13 2016, 1:14 PM
epriestley planned changes to this revision.Jun 13 2016, 1:17 PM

Fixing those other test issues.

epriestley added a child revision: D16100: Remove all uses of PhutilGitURI in Phabricator.Jun 13 2016, 1:17 PM
epriestley updated this revision to Diff 38737.Jun 13 2016, 1:34 PM
  • Fix ambiguous cases of "javascript:evil" and "mailto:user@domain".
  • Improve ease of debugging markup test case.
chad accepted this revision.Jun 13 2016, 2:18 PM
chad edited edge metadata.
This revision is now accepted and ready to land.Jun 13 2016, 2:18 PM
Closed by commit rPHU8bb124c37de3: Merge PhutilGitURI into PhutilURI (authored by epriestley, committed by epriestley). · Explain WhyJun 13 2016, 2:20 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
markup/
__tests__/
15 lines
parser/
180 lines
__tests__/
64 lines

Diff 38737

View Options

src/markup/__tests__/PhutilMarkupTestCase.php

Loading...
View Options

src/parser/PhutilURI.php

Loading...
View Options

src/parser/__tests__/PhutilURITestCase.php

Loading...
Phabricator · Built by Phacility