Page MenuHomePhabricator

[Wilds] Provide a skeleton for prompt behaviors
ClosedPublic

Authored by epriestley on Sep 25 2018, 4:15 PM.

Details

Summary

Ref T13098. Ref T13198. Ref T12996. The major ideas here are:

Workflows must define a list of all the prompts they can raise, so that these prompts can be enumerated with arc prompts <workflow>.

Prompts themselves should respond properly to ^C (abort immediately) and we should be able to make them nonblocking in the future (particularly, we'd like to be able to continue reading bytes from subprocess buffers while the prompt is shown on screen).

This doesn't have a lot of fancy features yet (non-confirm prompts, default yes, prompts which don't abort on "N", etc) but those should be easy to add later.

In the future, you'll be able to configure a default answer to prompts either in a config file or at runtime with --config prompts=x.y.z=N or similar.

This removes the history/readline mode for prompts, where you could use the up arrow to cycle through older responses. I believe this was only really useful for "excuse" prompts and intend to remove those.

Test Plan

Forced arc shell-complete to always prompt, then:

  • Got prompted, answered "y", "n", "N", "", "quack". Got sensible behavior in all cases.
  • Ran echo | arc shell-complete, got a TTY error.
  • Ran arc prompts, arc prompts shell-complete.

Diff Detail

Repository
rARC Arcanist
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Sep 25 2018, 4:15 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Sep 25 2018, 4:16 PM
Harbormaster failed remote builds in B20898: Diff 47084!
amckinley accepted this revision.Sep 26 2018, 5:09 PM
amckinley added inline comments.
src/toolset/ArcanistPrompt.php
84

I know this is a WIP, but is this is temporary debugging code, right?

135–143

Eventually this is going to be something more like isset($prompt_possible_answers[$response]), right?

src/toolset/ArcanistWorkflow.php
222–248

Yes, exactly like this!

This revision is now accepted and ready to land.Sep 26 2018, 5:09 PM
epriestley marked 2 inline comments as done.Sep 26 2018, 6:00 PM
epriestley added inline comments.
src/toolset/ArcanistPrompt.php
84

This is how it currently works today (i.e., on master). The idea is that if you see this:

$ echo | arc something
ERROR: arc is trying to prompt you but stdin is not a TTY.

...it's less helpful than this in figuring out what's going on:

$ echo | arc something
Continue even though the moon is full (spooky)? [y/N]
ERROR: arc is trying to prompt you but stdin is not a TTY.

I do plan to clean this up, though, and show you which prompt key you hit and give you next steps on pre-answering it once those pieces work.

135–143

Yeah -- right now there's only one "y = continue, n = abort" prompt on this new system, but I expect we'll support various other prompts as workflows get updated. Today I think we have "y = one option, n = something else, both options continue the workflow", "provide some text", and maybe "select from a set of options" prompts elsewhere.

This revision was automatically updated to reflect the committed changes.