Page MenuHomePhabricator

Update documentation for "uri.allowed-protocols"
ClosedPublic

Authored by epriestley on Mon, Apr 15, 6:59 PM.

Diff Detail

Repository
rP Phabricator
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.Mon, Apr 15, 6:59 PM
epriestley requested review of this revision.Mon, Apr 15, 7:01 PM
avivey accepted this revision.Mon, Apr 15, 7:43 PM
avivey added a subscriber: avivey.
avivey added inline comments.
src/applications/config/option/PhabricatorSecurityConfigOptions.php
178

maybe pht should know %n for newline?

This revision is now accepted and ready to land.Mon, Apr 15, 7:43 PM
This revision was automatically updated to reflect the committed changes.
epriestley added inline comments.Tue, Apr 16, 12:33 PM
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.

avivey added inline comments.Tue, Apr 16, 5:49 PM
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. ๐Ÿคทโ€โ™‚๏ธ