Paths

Table of Contentst

Differential D20445

Make chart function argument parsing modular/flexible with 900 pages of error messages
ClosedPublic
Actions

Authored by epriestley on Apr 17 2019, 6:29 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Fri, Feb 14, 10:50 PM

	Unknown Object (File)
	Fri, Feb 14, 10:50 PM

	Unknown Object (File)
	Fri, Feb 14, 10:50 PM

	Unknown Object (File)
	Fri, Feb 14, 10:50 PM

	Unknown Object (File)
	Fri, Feb 14, 10:50 PM

	Unknown Object (File)
	Fri, Feb 14, 10:50 PM

	Unknown Object (File)
	Tue, Feb 11, 6:23 AM

	Unknown Object (File)
	Sun, Feb 9, 7:40 AM

Subscribers

Details

Reviewers

Maniphest Tasks

T13279: Build Charting for Facts

Commits

rPedaf17f3fe51: Make chart function argument parsing modular/flexible with 900 pages of error…

Summary

Depends on D20444. Ref T13279. Instead of ad-hoc parsing and messages, formalize chart function arguments.

Also, add a whole lot of extra type checking.

Test Plan

Built and charted various functions with various valid and invalid argument lists, got sensible-seeming errors and results.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Apr 17 2019, 6:29 PM

Herald added a subscriber: yelirekim. · View Herald TranscriptApr 17 2019, 6:29 PM

Harbormaster completed remote builds in B22623: Diff 48753.Apr 17 2019, 6:30 PM

epriestley requested review of this revision.Apr 17 2019, 6:30 PM

Make the "constant" function actually read its argument.

Harbormaster completed remote builds in B22624: Diff 48754.Apr 17 2019, 6:36 PM

This feature is really coming along, practically ready to ship!

Screen Shot 2019-04-17 at 11.44.11 AM.png (1×1 px, 217 KB)

epriestley added a child revision: D20446: Simplify implementation of "pure" Chart functions.Apr 17 2019, 10:08 PM

amckinley accepted this revision.Apr 18 2019, 11:37 PM

amckinley added inline comments.

src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php
130	"requesting"
src/applications/fact/chart/PhabricatorXChartFunction.php
9	Just for the sake of code coverage, have you tried making any functions accept more than one argument? There's a lot of fancy looping/iterating in this diff that I don't think is getting put through its paces by the existing 0/1-argument functions.

This revision is now accepted and ready to land.Apr 18 2019, 11:37 PM

In a future diff, scale() and shift() take two arguments and work, although I'm likely rethinking exactly how the composition/chaining part of this works (see D20452 for rambling).

Typo fix.

Harbormaster completed remote builds in B22703: Diff 48828.Apr 23 2019, 7:23 PM

Closed by commit rPedaf17f3fe51: Make chart function argument parsing modular/flexible with 900 pages of error… (authored by epriestley). · Explain WhyApr 23 2019, 7:26 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

__phutil_library_map__.php

applications/

fact/

chart/

PhabricatorChartFunction.php

46 lines

PhabricatorChartFunctionArgument.php

129 lines

PhabricatorChartFunctionArgumentParser.php

154 lines

PhabricatorConstantChartFunction.php

28 lines

PhabricatorFactChartFunction.php

37 lines

PhabricatorSinChartFunction.php

30 lines

PhabricatorXChartFunction.php

9 lines

controller/

PhabricatorFactChartController.php

3 lines

fact/

PhabricatorFact.php

4 lines

Diff 48829

src/__phutil_library_map__.php

Loading...

src/applications/fact/chart/PhabricatorChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorChartFunctionArgument.php

Loading...

src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php

Loading...

src/applications/fact/chart/PhabricatorConstantChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorFactChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorSinChartFunction.php

Loading...

src/applications/fact/chart/PhabricatorXChartFunction.php

Loading...

src/applications/fact/controller/PhabricatorFactChartController.php

Loading...

src/applications/fact/fact/PhabricatorFact.php

Loading...