Details
Read config.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
src/applications/config/option/PhabricatorSecurityConfigOptions.php | ||
---|---|---|
178 | maybe pht should know %n for newline? |
src/applications/config/option/PhabricatorSecurityConfigOptions.php | ||
---|---|---|
178 | Currently, pht() resolves more or less as an unmodified sprintf(), and PHP sprintf() doesn't know "%n", so we'd have to do at least a bit more mangling than we currently do (versus qsprintf(), which uses xsprintf() to parse the pattern and which we can add new "%" rules to pretty easily). I'd be slightly worried about the performance cost to parse the pattern at runtime, although this particular rule could probably be safely implemented with str_replace() since it doesn't actually consume or interact with arguments. We might also need to clean this up on the bin/i18n extract pathway, but this is doable. Mostly, it's not obvious to me that "%n" is dramatically better than "\n", even though this "\n\n". construction is pretty awkward. We'd still probably end up with '%n%n'. as the most visually clear representation of the string, I think? Dropping the mixed single-quoted and double-quoted strings would be a little bit nice, but this doesn't seem like a huge step forward unless I'm misunderstanding? A bigger win here might be teaching PHP some kind of better HEREDOC to get rid of ALL the string quoting nonsense, although I think this is a dangerous path to walk down. We could also put blocks like this in some kind of external file, but then we need to teach a bunch of editors how to interact with that sensibly. |
src/applications/config/option/PhabricatorSecurityConfigOptions.php | ||
---|---|---|
178 | yeah, I was thinking we would inline %n%n into the preceding paragraph and get rid of both mixed quote and extra line, but it's probably more clear to just have it separate anyway. 🤷♂️ |