Page MenuHomePhabricator

Improve parsing of docblock specials
ClosedPublic

Authored by joshuaspence on Aug 23 2016, 12:26 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 28, 7:34 PM
Unknown Object (File)
Thu, Apr 25, 12:00 AM
Unknown Object (File)
Tue, Apr 16, 5:52 PM
Unknown Object (File)
Thu, Apr 11, 8:18 AM
Unknown Object (File)
Fri, Apr 5, 6:19 PM
Unknown Object (File)
Apr 1 2024, 5:35 PM
Unknown Object (File)
Mar 31 2024, 12:39 PM
Unknown Object (File)
Mar 30 2024, 1:46 AM
Subscribers

Details

Summary

Currently the tags of following docblock are parsed as array('special' => "foo\nbar"). This diff changes the output to be array('special' => array('foo', 'bar')).

/**
 * @special foo
 * @special bar
 */
Test Plan

Ran unit tests.

Diff Detail

Repository
rPHU libphutil
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 13395
Build 17197: Run Core Tests
Build 17196: arc lint + arc unit

Event Timeline

joshuaspence retitled this revision from to Improve parsing of docblock specials.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

One inline -- and what does this produce (is there a test for it)?

@key value
@key
src/parser/PhutilDocblockParser.php
121

This seems a little odd to me, I'd expect:

@morse-code dot
@morse-code dot
@morse-code dash

...or whatever to produce array('dot', 'dot', 'dash'). Is there a particular reason for it?

src/parser/PhutilDocblockParser.php
121

Right, this was primarily intended for this case:

@special
@special
@special

In this case, I figured true was better than array(true, true, true).

joshuaspence edited edge metadata.

Add another test case

Ah, that makes sense for the no-strings case. Agreed that true is probably the right value there.

(I'm actually not really sure what I expect string + no string case to do.)

epriestley edited edge metadata.

Cool, those behaviors seem reasonable to me. Thanks!

(Does @stable @stable @stable have a test case?)

This revision is now accepted and ready to land.Aug 23 2016, 2:01 AM
joshuaspence edited edge metadata.

Add another test case

This revision was automatically updated to reflect the committed changes.