Page MenuHomePhabricator

Allow specifying runtime configuration with --set-config key=value
ClosedPublic

Authored by talshiri on Jun 9 2014, 10:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 12:48 PM
Unknown Object (File)
Fri, Nov 22, 9:17 AM
Unknown Object (File)
Fri, Nov 22, 9:17 AM
Unknown Object (File)
Fri, Nov 22, 8:38 AM
Unknown Object (File)
Fri, Nov 22, 7:53 AM
Unknown Object (File)
Thu, Nov 21, 1:31 PM
Unknown Object (File)
Thu, Nov 7, 11:44 PM
Unknown Object (File)
Oct 26 2024, 4:18 AM
Subscribers

Details

Summary

This is useful for wrapper scripts that want to customize arcanist's behavior without affecting the global configuration.
This can be implemented with arcanist_configuration entry in .arcconfig, however it is currently limited to
per-project settings, and this feature makes writing wrapper scripts a little easier.

Test Plan

arc diff --set-config editor=vim (yeah yeah, crappy test case)

Diff Detail

Repository
rARC Arcanist
Branch
runtime_config
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1042
Build 1042: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

talshiri retitled this revision from to Allow specifying runtime configuration with --set-config key=value.
talshiri updated this object.
talshiri edited the test plan for this revision. (Show Details)
talshiri added a reviewer: epriestley.
talshiri edited edge metadata.

--set-config -> --config

epriestley edited edge metadata.

This generally looks good, but some inlines around dealing with edge cases.

scripts/arcanist.php
38–42

You should parse this new flag here, with PhutilArgumentParser, instead of as part of workflows.

In particular, some options may affect which workflows are available, like load. If we wait until we have a workflow to parse arguments, it will become impossible to run arc --config load=xxxx custom-workflow y z.

src/workflow/ArcanistBaseWorkflow.php
630

I think this will raise a (confusing) warning/fatal if there's no = in the argument.

Instead, we should throw ArcanistUsageException.

631

This should use ArcanistSettings->willWriteValue() to interpret the value (the name is a little confusing, but willWriteValue() is the one that takes a user-entered value and converts it into a machine-readable value).

Otherwise, it's impossible to configure lists/maps, and values like "true", "false", "on" and "off" will not be interpreted correctly.

This revision now requires changes to proceed.Jun 16 2014, 6:48 PM
talshiri edited edge metadata.
  • moved config parsing to pre-workflow phase
  • clean up patch a bit
talshiri edited edge metadata.
  • show up in help --full
epriestley edited edge metadata.

One minor spelling issue, I'll just clean that up in the pull.

scripts/arcanist.php
51

"invocation"

This revision is now accepted and ready to land.Jun 16 2014, 10:27 PM
epriestley updated this revision to Diff 22972.

Closed by commit rARC5a02012706b5 (authored by @talshiri, committed by @epriestley).