See T12266. Warn when the user tries to set a value we will probably not read.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rARCbc9b70508edf: arc set-config: warn about unknown keys
arc set-config foo bar, see fancy warning.
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 15636 Build 20639: Run Core Tests Build 20638: arc lint + arc unit
Event Timeline
| src/workflow/ArcanistSetConfigWorkflow.php | ||
|---|---|---|
| 79 | I think you can make this translatable like this: $warning = tsprintf( "**%s:** %s\n", pht('Warning'), pht('The configuration key ...')); $console->writeErr("%s", $warning); | |
| src/workflow/ArcanistSetConfigWorkflow.php | ||
|---|---|---|
| 103 | Hmm, now these strings aren't translatable. | |
Comment Actions
pht() is the only thing that does translations right now.
Omitting tsprintf() means that ANSI stuff won't be escaped. It doesn't really matter here since the string is controlled by the user, but there are quasi-attacks where users create a task or revision with a lot of terminal control characters in it so that arc list or arc todo or whatever potentially take over your terminal. tsprintf() avoids that.
The overall state of affairs isn't great right now since the minimal amount of code you need to write the word "hello" to stderr "correctly" (so it can be translated and is ANSI-safe) is this big mess:
$console->writeErr( "%s", tsprintf( "%s\n", pht('hello')));
This is pretty silly, but since pht() needs to be statically extracted we can't easily get rid of it, and merging the $console stuff with tsprintf() is sort of half-pointless if we don't go all the way to try to get rid of the pht() too.
There's also at least some kind of argument that "\n" and ANSI sequences shouldn't be translatable: at best, they're probably pretty confusing.