Paths

Table of Contentst

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

Subscribers

None

Details

Reviewers

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

Branch

gituri1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 12608
Build 16005: Run Core Tests
Build 16004: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	0 ms	PhutilMarkupTestCase::Unknown Unit Message ("") Assertion failed, expected values to be equal (at PhutilMarkupTestCase.php:179): Rejected href: Expected: true Actual: false
	27 ms	PhutilRemarkupEngineTestCase::Unknown Unit Message ("") Assertion failed, expected values to be equal (at PhutilRemarkupEngineTestCase.php:63): Failed to markup HTML in file 'link-mailto.txt'. Expected vs Actual Output Diff --- Old Value
	1 ms	AbstractDirectedGraphTestCase::Unknown Unit Message ("") 25 assertions passed.
	0 ms	AbstractDirectedGraphTestCase::Unknown Unit Message ("") 1 assertion passed.
	0 ms	AbstractDirectedGraphTestCase::Unknown Unit Message ("") 1 assertion passed.
		View Full Test Results (2 Failed · 341 Passed · 2 Skipped)

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.

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.

Remove "INVALID_PATH" faux-error.

epriestley mentioned this in D16100: Remove all uses of PhutilGitURI in Phabricator.Jun 13 2016, 1:14 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

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

Path

Size

src/

parser/

138 lines

__tests__/

PhutilURITestCase.php

32 lines

Diff 38733

src/parser/PhutilURI.php

Loading...

src/parser/tests/PhutilURITestCase.php

Loading...