Modularize parser for "Closes task X as Y"
ClosedPublic
Actions

Authored by epriestley on Feb 17 2014, 7:01 PM.

Details

Reviewers

btrahan

Maniphest Tasks

Restricted Maniphest Task
T1812: Allow for arbitrary values for task status
Restricted Maniphest Task

Commits

Restricted Diffusion Commit
rPd016cac91513: Modularize parser for "Closes task X as Y"

Summary

Ref T3886. Ref T3872. Ref T1812. We have several parsers which look for textual references to other objects, like:

Closes Tx.
Depends on Dy.
Reverts Dz.

Currently, these are pretty hard coded, don't get all the edge cases right, and don't generalize well. They're also implemented in the middle of Differential's field code. So I want to:

Share more code so that, e.g., "Tx, Ty" always works (only some rules support it right now);
fix bugs in the parser, like T3872;
make this a modular, extensible process which runs against custom fields, not a builtin part of fields;
make the internals more flexible to accommodate custom stuff like T1812.

This implements the "Verbs optional-noun Object, Optional Other Objects optional-as-something." grammar in a general way so subclasses can just plug in their keywords. Runtime code doesn't touch this yet.

Test Plan

Ran unit tests.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

I'm intentionally ignoring internationalization here for now. This positions us better for it than we were positioned before, but if we pht() these strings it prevents other languages from having more or fewer trigger phrases. We could do something like this:

$phrases = pht('a,b,c,d');
return explode(',', $phrases);

However, this would probably be pretty non-obvious to translators. We could do this:

$phrases = pht('Enter a comma-separated list of prefix phrases for marking a dependency (English = a,b,c,d).');

...and then translate that to "a,b,c,d" in English, but I'd rather wait until someone actually wants to translate this and/or we get more similar cases before doing that, since it feels a bit rough.