Placeholders and delimiters

[Paul Jones]

Hi all,

(This message is a result of my very strong desire to have a discussion related to placeholders and delimiters in the LoggerInterface.)


Named or sequential placeholders?
---------------------------------

I would argue in favor of *only* named placeholders, so that we can easily throw an array of context data against a message, where the array keys do not need to be in any particular order.

I would argue also that we don't need to define the placeholder formats; anything between the delimiters should be allowed.


What delimiters to use?
-----------------------

These are the ones I know are used by various projects, members or otherwise, whether for logging interpolation or for other string substitutions (please feel free to add to this list *from existing projects*).

Drupal:
%foo
@foo
!foo

ext/intl and possibly others:
{foo}

Lithium, Solar/Aura:
{:foo}

Twig, Mustache, et al.:
{{foo}} (no spaces allowed)
{{ foo }} (zero or more spaces allowed)

Suggested by Jordi:
%foo%

Different implementors will approach interpolation from different directions. Some will preg_match_all() the message string to find and replace placeholders, others will loop through $context and do a str_replace() on the message using the $context keys, others may do something else entirely.

With that in mind, I would first argue that we need an opening and closing delimiter, not just an opening delimiter. This makes it easier to search for placeholder names in the message; you don't need to define what characters are allowed in the names, just the delimiters.

I would further argue that they should be different sets of characters; e.g., {foo} and not %foo%. I think thinks makes it easier to search for multiple tokens on the same line; you only need to define the delimiters, not the placeholder names.

Finally, I would argue that whatever we come up with should be usable in short-message contexts other than logging: things like localized message placeholders, router placeholders, validation messages, etc.

That's all for now. Thoughts?


-- pmj

[Lukas Kahwe Smith]

On Dec 11, 2012, at 23:28 , Paul Jones <pmjo...@gmail.com> wrote:

> I would further argue that they should be different sets of characters; e.g., {foo} and not %foo%. I think thinks makes it easier to search for multiple tokens on the same line; you only need to define the delimiters, not the placeholder names.

I dont understand this. what makes "{foo}" a "different sets of characters" and not "%foo%". i am probably oblivious to the definition of character sets.

regards,
Lukas Kahwe Smith
m...@pooteeweet.org

[Evert Pot]

On Dec 11, 2012, at 11:52 PM, Lukas Kahwe Smith <m...@pooteeweet.org> wrote:

>
> On Dec 11, 2012, at 23:28 , Paul Jones <pmjo...@gmail.com> wrote:
>
>> I would further argue that they should be different sets of characters; e.g., {foo} and not %foo%. I think thinks makes it easier to search for multiple tokens on the same line; you only need to define the delimiters, not the placeholder names.
>
>
> I dont understand this. what makes "{foo}" a "different sets of characters" and not "%foo%". i am probably oblivious to the definition of character sets.

I can see that %foo%bar% could be problematic.

This wouldn't be a problem if there's an escaping mechanism though. Thoughts?

Evert

[Lukas Kahwe Smith]

ah get it now .. the term "set" confused me .. "different characters" seems clearer to me.

[Paul Jones]

Yeah that's fair -- I wanted to include the idea that it might be more than one character, such as {:foo} or {{foo}}. Sorry for the poor phrasing.


-- pmj

[Paul Jones]

On Dec 11, 2012, at 4:56 PM, Evert Pot wrote:

> I can see that %foo%bar% could be problematic.
>
> This wouldn't be a problem if there's an escaping mechanism though. Thoughts?

/me nods

To be clear, you're saying we should be able to escape the delimiters so they are not interpreted as placeholders, e.g. using a backslash as part of the opening delimiter? So, {foo} would be a placeholder, and \{foo} would not be -- yes?


-- pmj

[Evert Pot]

Exactly.

I feel that if I'm going to log a lot of debugging data, it would be beneficial if I can run it through an escaping function to ensure that now substitution never happens.

If we're escaping with a backslash (and \\ for a 'raw' backslash), using %key% would also no longer be ambiguous.

Evert

[Florin Patan]

Sorry that I haven't read the whole Logging problem but it's waaay to long to catch up with it now but why do we need to have named placeholders in the logging interface/component?

Shouldn't the placeholders for sprintf() be enough, with %s for strings and so on?

I think we are over complicating something that should be by default a simple thing to do in very very very __fast__ manner. I've seen that translation was one issue but in this case, the only thing about logs + translations makes me thing of a single, very specific case, where you want to display the logs to the end users, who don't speak English/native language of the app developer. And even so, I think translators are generally smart guys/girls and can understand that %s means a placeholder or that a context can be added to the translation string to better determine what should go into that placeholder instead of %{{!@className@!}}% only. For translation purpose I'd see that there's a need to specify what are you trying to have translated rather that give them generic placeholders if you want the translation to be more accurate.

If this has been suggested and dismissed, I'm sorry, I'll reserve a couple of hours in the weekend to read the whole lists (I think this is the 5th one related to Logging by now?)

Thanks for your time.

Best regards. 

[Beau Simensen]

For what it is worth, I have a library specifically designed to handle resolving placeholders. It works as easily with '${placeholder}' as it does with '%placeholder%'. It ships with an array data source implementation that could take a context array and use it outright.

 * https://github.com/dflydev/dflydev-placeholder-resolver

[William Durand]

I tend to agree with Florin. Either we should drop this placeholder/delimiter thing to avoid PlaceholderAbstractFactory and so on, or we should keep it simple as Jordi suggested.

According to Paul's survey, it's not that used anyway. Just my point of view though...

William

--

William Durand | http://www.williamdurand.fr

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/php-fig/-/OIKlVvaJMk0J.

For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Matthew Lanigan]

Down this road lies madness. But in all seriousness, the addition of an escape character like that makes both string replacement and simple regular expression-replacement unfeasible strategies, so I would advocate against the addition of an escape -- unless there is a simple solution that I'm missing.

Matthew

>
> Evert

>
> --
> You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
> To post to this group, send email to php...@googlegroups.com.
> To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.

[Moisa Teodor]

Hi all,

I would also add a sprintf-like syntax (sequential placeholders, formatting) to the list because:

 - it is used by at least one project represented in this group

 - the sprintf syntax and formatting specifiers are known by everyone

 - some formatting stuff like "%42s" would work out of the box (could increase readabiliy in some cases; would also make it very easy to satisfy some corporate log file parsers if used with logging )

 - it's very easy to write a quick and dirty wrapper that just calls vsprintf

However, there are disadvantages:

 - cannot easily use the same value more than once (the sprintf syntax is quite unfriendly, ex "%1$s" - to re-use the first argument) 

 - no support for named placeholders

Another option would be to have a combination of the sprintf capabilities and one of the other options in the list; something like {:placeholder, format}, where the format could have a sprintf-like syntax. The format syntax could also be extended (plugins ?) to support for example password shadowing or other formatting needs.

What do you think ?

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Doru Moisa
web: http;//moisadoru.wordpress.com
tel: +40 720 861 922
Bucharest, Romania

[Evert Pot]

On Dec 12, 2012, at 12:39 AM, Matthew Lanigan <rin...@gmail.com> wrote:

On Dec 11, 2012 6:04 PM, "Evert Pot" <ever...@gmail.com> wrote:
>
> On Dec 12, 2012, at 12:00 AM, Paul Jones <pmjo...@gmail.com> wrote:
>
> >
> > On Dec 11, 2012, at 4:56 PM, Evert Pot wrote:
> >
> >> I can see that %foo%bar% could be problematic.
> >>
> >> This wouldn't be a problem if there's an escaping mechanism though. Thoughts?
> >
> > /me nods
> >
> > To be clear, you're saying we should be able to escape the delimiters so they are not interpreted as placeholders, e.g. using a backslash as part of the opening delimiter?  So, {foo} would be a placeholder, and \{foo} would not be -- yes?
>
> Exactly.
>
> I feel that if I'm going to log a lot of debugging data, it would be beneficial if I can run it through an escaping function to ensure that now substitution never happens.
>
> If we're escaping with a backslash (and \\ for a 'raw' backslash), using %key% would also no longer be ambiguous.

Down this road lies madness. But in all seriousness, the addition of an escape character like that makes both string replacement and simple regular expression-replacement unfeasible strategies, so I would advocate against the addition of an escape -- unless there is a simple solution that I'm missing.

A regex like this would do the trick I believe (untested):

(

  \\ |

  \% |

  %([^%]+)%

)

with preg_replace_callback, this is not terribly hard to implement. I bet this can be optimized a bit though, by never calling the callback for the first two cases.

Evert

[Paul Jones]

On Dec 11, 2012, at 5:40 PM, Moisa Teodor wrote:

> Hi all,
>
> I would also add a sprintf-like syntax (sequential placeholders, formatting) to the list because:
> - it is used by at least one project represented in this group

Ah, my mistake -- the sprintf() in Buzz is not for logging, per se, but for formatting a message that goes to a LoggingListener. I'll update the survey accordingly.


-- pmj

[Paul Jones]

On Dec 11, 2012, at 5:46 PM, Evert Pot wrote:

> A regex like this would do the trick I believe (untested):
>
> (
> \\ |
> \% |
> %([^%]+)%
> )
>
> with preg_replace_callback, this is not terribly hard to implement. I bet this can be optimized a bit though, by never calling the callback for the first two cases.

/me nods

Note, though, that the only use of %foo% was from Jordi; no offense, but that's not in use in any of the member projects for any interpolation purpose that I'm aware of.


-- pmj

[Evert Pot]

I have no preference either way :) I do feel that if the decision falls in favour of using substitution at all, it would be wise to allow for escaping in any case.

Evert

[Beau Simensen]

%foo% is used quite heavily by Symfony. Maybe not in logging
specifically but all of the parameter placeholders use this format.

Sent from my iPhone

[Matthew Lanigan]

On Tue, Dec 11, 2012 at 6:46 PM, Evert Pot <ever...@gmail.com> wrote:

A regex like this would do the trick I believe (untested):

(

  \\ |

  \% |

  %([^%]+)%

)

with preg_replace_callback, this is not terribly hard to implement. I bet this can be optimized a bit though, by never calling the callback for the first two cases.

Ah, I stand corrected -- but I do maintain that escape characters complicate the process significantly.

As a side note, your regex works, but there are some fun edge cases -- e.g. ("\%foo% bar \%baz%") matches the key " bar \", which I find humorous. :)

Matthew

[Paul Jones]

On Dec 11, 2012, at 6:16 PM, Beau Simensen wrote:

> %foo% is used quite heavily by Symfony. Maybe not in logging
> specifically but all of the parameter placeholders use this format.

Ah, good enough. Thanks!


-- pmj

[Matthew Lanigan]

On Tue, Dec 11, 2012 at 5:28 PM, Paul Jones <pmjo...@gmail.com> wrote:

I would argue in favor of *only* named placeholders, so that we can easily throw an array of context data against a message, where the array keys do not need to be in any particular order.

I agree with this; named placeholders do still allow for integer-valued placeholders (e.g. "{{0}}") -- if someone has their heart set on sequential placeholders -- as arrays can have integer keys. I'm not sure I see the benefit of doing that, but to each his or her own.

 

I would argue also that we don't need to define the placeholder formats; anything between the delimiters should be allowed.

I have to disagree here, however. I believe that some bare minimum specification of placeholder formats is absolutely required; specifically, "placeholders may not contain the closing delimiter" or some similar restriction. Though escaping the opening delimiter is relatively easy, allowing key-internal escapes (i.e. not restricting keys from containing the closing delimiter) is much more complicated, and by omitting a restriction to this effect, we are condemning implementors to that complication.

 

With that in mind, I would first argue that we need an opening and closing delimiter, not just an opening delimiter. This makes it easier to search for placeholder names in the message; you don't need to define what characters are allowed in the names, just the delimiters.  

 

I would further argue that they should be different sets of characters; e.g., {foo} and not %foo%. I think thinks makes it easier to search for multiple tokens on the same line; you only need to define the delimiters, not the placeholder names.

I'm not sure how much this actually makes things easier, or if having different characters for opening and closing delimiters, so I'm fine with anything here.

 

Finally, I would argue that whatever we come up with should be usable in short-message contexts other than logging: things like localized message placeholders, router placeholders, validation messages, etc.

I agree that this should apply everywhere that placeholder interpolation is relevant, not just in logging. As we saw in your survey, placeholders are actually very rarely used in logging facilities themselves; however, they are used elsewhere.

I think it is important that this be considered separate, though absolutely related, functionality to the LoggerInterface proposal.

Matthew

[Paul Jones]

On Dec 11, 2012, at 6:46 PM, Matthew Lanigan wrote:

> On Tue, Dec 11, 2012 at 5:28 PM, Paul Jones <pmjo...@gmail.com> wrote:
>> I would argue in favor of *only* named placeholders, so that we can easily throw an array of context data against a message, where the array keys do not need to be in any particular order.
>
> I agree with this; named placeholders do still allow for integer-valued placeholders (e.g. "{{0}}") -- if someone has their heart set on sequential placeholders -- as arrays can have integer keys. I'm not sure I see the benefit of doing that, but to each his or her own.

Good point.

>> I would argue also that we don't need to define the placeholder formats; anything between the delimiters should be allowed.
>
> I have to disagree here, however. I believe that some bare minimum specification of placeholder formats is absolutely required; specifically, "placeholders may not contain the closing delimiter" or some similar restriction. Though escaping the opening delimiter is relatively easy, allowing key-internal escapes (i.e. not restricting keys from containing the closing delimiter) is much more complicated, and by omitting a restriction to this effect, we are condemning implementors to that complication.

Rats! Just when I think it's going to easier. :-/ So yes, at a minimum, we do need to define that. (Although I suppose in practice that'll be easy, in that you search up to the ending token delimiter and don't allow for escaping.)

>> Finally, I would argue that whatever we come up with should be usable in short-message contexts other than logging: things like localized message placeholders, router placeholders, validation messages, etc.
>
> I agree that this should apply everywhere that placeholder interpolation is relevant, not just in logging. As we saw in your survey, placeholders are actually very rarely used in logging facilities themselves; however, they are used elsewhere.
>
> I think it is important that this be considered separate, though absolutely related, functionality to the LoggerInterface proposal.

/me nods


-- pmj

[Evert Pot]

It was quick and dirty :P I agree it should probably match " bar \% baz" in that case

Evert

[Alexandre Gaigalas]

Just some thoughts...

While developing http://github.com/Respect/Validation we needed a placeholder for templated error messages. Initially we used sprintf but ordered placeholders are insane, we really needed named ones. There is no native PHP function for that (we would have chosen it over anything), so we settled on the most compatible, historically relevant and working solution with the minimal implementation possible. Our placeholders are based on the Twig style and formatted with this function:

    public static function format($template, array $vars=array())
    {
        return preg_replace_callback(
            '/{{(\w+)}}/',
            function($match) use($vars) {
                return isset($vars[$match[1]]) ? $vars[$match[1]] : $match[0];
            }, $template
        );
    }

Any delimiter is easy to implement based on this. I would personally choose the Twig style for standardization for the same reasons we considered on Respect\Validation: most compatible, historically relevant, easy to implement, easy to customize and empirically comproved.

I'm really interested on the development of this. Ready to help in anything if you guys need.

--
Alexandre Gaigalas

[Paul Jones]

On Dec 11, 2012, at 8:12 PM, Alexandre Gaigalas wrote:

> Our placeholders are based on the Twig style and formatted with this function:
>
> public static function format($template, array $vars=array())
>
> {
> return preg_replace_callback(
> '/{{(\w+)}}/',
>
> function($match) use($vars) {
>
> return isset($vars[$match[1]]) ? $vars[$match[1]] : $match[0];
>
> }, $template
> );
> }

Nice. The Twig/Mustache/Handlebars/etc double-braces notation does seem very common across disparate projects in the wild. It's certainly more common than the {:foo} I'm biased toward, and maybe even more common than single braces.

I hesitate to bring this up, but ...

Some systems, such as Aura and Lithium (and probably others) sometimes allow added information after the placeholder name. For example, `{:id:(/d+)}` (in this case to specify a regular expression for the 'id' parameter in a routing definition). Likewise, some template systems will have a pipe character and a series of filters to apply: `{{foo|trim|html}}`.

With that in mind, and with the idea that the placeholder/delimiter system should be usable for more than just variable interpolation in logging systems, would it make sense to modify the regex to something like the following?

/{{(\w+)(.*)?}}/

That allows for the placeholder name, plus whatever variation of optional follow-on specifications that varying libraries might need.

Like I said, I hesitate to bring it up at this point, but as long as we're here.


-- pmj

[Alexandre Gaigalas]

Never found a really good solution for that.

Wish we had something similar to str.format() from Python (which is built in and supports named placeholders). Their spec is pretty nice: http://docs.python.org/2/library/string#formatstrings http://docs.python.org/2/library/string#formatspec

Capturing these modifiers with a regex seems straightforward (like you showed), but post-processing them to then re-format every parameter doesn't sound nice. sprintf already does the formatting by modifiers pretty well (in C!), it just lacks named placeholders.

In my opinion the ideal solution considering the modifiers would be something that reuses a single call to sprintf and adds named parameter support. Can be done with sprintf ordered parameters (%1$s), extending it's syntax with a shallow parser. Did a simple sample implementation (largely untested, slightly illegible):

function format($template, $vars) {
    $keys = array_keys($vars);
    $template = preg_replace_callback(
        '/\%(\w+)\$/',
        function ($match) use (&$keys) {
            return "%" . (array_search($match[1], $keys) + 1) . "$";
        },
        $template
    );
    return vsprintf($template, $vars);
}

print format('Hello %bar$s %foo$05d', array('foo' => 42, 'bar' => 'caipirinha'));

The sprintf default modifiers should be enough for most of the cases (except callbacks and things that aren't plain string formatting).

Any thoughts?

-- pmj

[FGM at GMail]

FWIW, transforming a whole set of placeholders is just a single call
to strtr(), not so different from a single call to sprintf, just much
more readable.

2012/12/12 Alexandre Gaigalas <alex...@gaigalas.net>:

[Paul Jones]

On Dec 12, 2012, at 7:29 AM, Alexandre Gaigalas wrote:

> Never found a really good solution for that.

Cool. I'm OK with leaving it out of the definition.

> In my opinion the ideal solution considering the modifiers would be something that reuses a single call to sprintf and adds named parameter support.

Well, we're not doing implementations per se, although writing example implementations can help us figure out what makes sense for a spec.

* * *

In the LoggingIterface thread, Jordi has opined that he's ok with single brace separators. I think that's a move in the right direction.

There's also the Drupal guys, who use a single leading non-word character to indicate what kind of escaping needs to happen on the placeholder.

With those in mind, I'd like to suggest the following as a general compromise:

- Placeholder delimiters MUST use a single opening brace and a single closing brace, with no whitespace between the delimiters and the name

- Placeholder names MUST be one or more word characters

- Placeholders MAY optionally have a single non-word non-whitespace character between the opening brace and the placeholder name; this character MAY be used by the implementor as information about how to treat the replacement string.

That gives us a regular expression that looks something like this:

/{([^\w\s])?([\w]+)}/

(We use [^\w\s] instead of [\W\S] because we need negation of all the character classes; otherwise it evaluates the classes individually.)

This means that the Drupal guys, to implement the proposal, need only add braces around their placeholders.

Test code:

$list = [
// pass
'{foo}',
'{%foo}',
'{@foo}',
'{!foo}',
// fail
'{ foo}',
'{{ foo }}',
];

$find = "/{([^\w\s])?([\w]+)}/";

foreach ($list as $item) {
echo $item . ' ';
var_dump(preg_match($find, $item));
}

(The placeholder {{foo}} will pass, because the inner braces match the pattern, so the replacement would be surrounded by the external braces. Do we care about that?)

Thoughts?


-- pmj

[Matthew Lanigan]

On Wed, Dec 12, 2012 at 10:16 AM, Paul Jones <pmjo...@gmail.com> wrote:

(The placeholder {{foo}} will pass, because the inner braces match the pattern, so the replacement would be surrounded by the external braces. Do we care about that?)

Thoughts?

Actually, that sounds perfectly expected. Unfortunately, I believe what would match with the currently-proposed regex is "{{foo}", leaving just the trailing brace after interpolation. Perhaps we should modify the regular expression to exclude an opening brace from being the first non-whitespace, non-word character? e.g.:

 /{([^{\w\s])?([\w]+)}/

Let me know what you think,

Matthew

[Paul Jones]

On Dec 12, 2012, at 9:43 AM, Matthew Lanigan wrote:

> On Wed, Dec 12, 2012 at 10:16 AM, Paul Jones <pmjo...@gmail.com> wrote:
>> (The placeholder {{foo}} will pass, because the inner braces match the pattern, so the replacement would be surrounded by the external braces. Do we care about that?)
>>
>> Thoughts?
>
> Actually, that sounds perfectly expected. Unfortunately, I believe what would match with the currently-proposed regex is "{{foo}", leaving just the trailing brace after interpolation.

Mmm, so would {foo}}.

> Perhaps we should modify the regular expression to exclude an opening brace from being the first non-whitespace, non-word character? e.g.:
>
> /{([^{\w\s])?([\w]+)}/

The question is: does it matter enough to put it in the example regex, or is it enough to document that this is an expected behavior?

I'm leaning toward the "documentation" approach; it's my guess that some folks will have a legitimate need to have surrounding braces.

You?


-- pmj

[Beau Simensen]

The question is: does it matter enough to put it in the example regex, or is it enough to document that this is an expected behavior? 

I can see how documenting using regex character classes might be nice but it feels like defining "the" regex falls into the implementation detail category. In that case I do not think it needs to be defined unless the intention is to provide this functionality in the package somehow. For example, something like: Psr\Log\LogMessage::format($string, $context)

[Paul Jones]

On Dec 12, 2012, at 10:27 AM, Beau Simensen wrote:

>> The question is: does it matter enough to put it in the example regex, or is it enough to document that this is an expected behavior?
>
> I can see how documenting using regex character classes might be nice but it feels like defining "the" regex falls into the implementation detail category. In that case I do not think it needs to be defined unless the intention is to provide this functionality in the package somehow.

I see exactly where you're coming from. My only reply is that, if it gets included, it would need to be explicitly labeled as an example for illustrative purposes only, much the same way the PSR-0 document includes an example implementation:

<https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md#example-implementation>


-- pmj

[Jason Judge]

On Wednesday, 12 December 2012 16:27:32 UTC, Beau Simensen wrote:

The question is: does it matter enough to put it in the example regex, or is it enough to document that this is an expected behavior? 

I can see how documenting using regex character classes might be nice but it feels like defining "the" regex falls into the implementation detail category. In that case I do not think it needs to be defined unless the intention is to provide this functionality in the package somehow. For example, something like: Psr\Log\LogMessage::format($string, $context)

Specification by regex sounds good in theory: it is robust, clear, defines exactly what is needed. The problem comes with bugs and edge-cases. There will always be *some* edge-case that is overlooked, e.g. a multi-byte characterset not taken into account or some glaring bug in the regex that is simply not spotted.

[Larry Garfield]

On 12/12/2012 09:16 AM, Paul Jones wrote:
>> In my opinion the ideal solution considering the modifiers would be something that reuses a single call to sprintf and adds named parameter support.
> Well, we're not doing implementations per se, although writing example implementations can help us figure out what makes sense for a spec.

Ordered placeholders are useless. Full stop. And if we're adding named
placeholders to sprintf(), then we're not using sprintf() anymore so
there's no programmatic benefit to modeling on sprintf().

>
> * * *
>
> In the LoggingIterface thread, Jordi has opined that he's ok with single brace separators. I think that's a move in the right direction.

Another data point: Although I don't believe we'd discussed using it in
the HTTP client thread, there is a draft IETF spec for URI patterns that
uses {}, which is what Symfony and now Drupal are using. So IF we were
to have placeholders in a future HTTP client thread, they'd probably be
using {}. I think that's a mark in {}'s favor.

> There's also the Drupal guys, who use a single leading non-word character to indicate what kind of escaping needs to happen on the placeholder.
>
> With those in mind, I'd like to suggest the following as a general compromise:
>
> - Placeholder delimiters MUST use a single opening brace and a single closing brace, with no whitespace between the delimiters and the name
>
> - Placeholder names MUST be one or more word characters

Should we qualify "word character" as "alphanumeric plus underscore?"
(I don't know if that's exactly the same thing, not being a regex guru.
Which probably means plenty of other people will interpret it that way,
or possibly just alphabetic characters, which will lead to confusion.
Let's be clearer on what characters are legal there.)

> - Placeholders MAY optionally have a single non-word non-whitespace character between the opening brace and the placeholder name; this character MAY be used by the implementor as information about how to treat the replacement string.
>
> That gives us a regular expression that looks something like this:
>
> /{([^\w\s])?([\w]+)}/
>
> (We use [^\w\s] instead of [\W\S] because we need negation of all the character classes; otherwise it evaluates the classes individually.)
>
> This means that the Drupal guys, to implement the proposal, need only add braces around their placeholders.
>
> Test code:
>
> $list = [
> // pass
> '{foo}',
> '{%foo}',
> '{@foo}',
> '{!foo}',
> // fail
> '{ foo}',
> '{{ foo }}',
> ];
>
> $find = "/{([^\w\s])?([\w]+)}/";
>
> foreach ($list as $item) {
> echo $item . ' ';
> var_dump(preg_match($find, $item));
> }
>
> (The placeholder {{foo}} will pass, because the inner braces match the pattern, so the replacement would be surrounded by the external braces. Do we care about that?)
>
> Thoughts?

While I appreciate you thinking about us :-), I don't think that will
work. Remember, Solarium and Guzzle are the target audience here, not
Drupal primarily.

But let's suppose for a moment that the Drupal Plugin component uses
this logging interface, and becomes available independently of Drupal
(which we'd like to do at some point). It uses {@foo} to indicate "you
escape this", {%foo} to indicate "you escape this, and wrap it in em
tags", and {!foo} to indicate "I already escaped this", because that's
what Drupal does now with its log and translation messages.

Now someone decides to use the Drupal Plugin component with Aura, and
passes in a log implementation. That log implementation now needs to
not only conform to LogInterface, its implementation needs to know what
to do with @, %, and !. If it doesn't, then it will blindly
double-escape {!foo} and won't know to wrap {%foo} in em tags. So you
can't just "drop in and go", you need a Drupal-specific LogInterface
implementation in order to use Drupal loggable components. Which is
exactly what we're trying to NOT have.

To avoid that, Drupal would need to have two different internal logging
standards, one for components it plans to release (which we don't always
know when they're being written) that eschews prefix format characters,
and one that's for components we are not going to release that can use
extended characters. Drupal already has too many
multiple-internal-standards, and I've no desire to encourage more. :-)

That does mean that if Drupal (or a similar project like ezPublish,
CakePHP, Aura, or whatever else is represented on this list) wants to
use LogerInterface itself, we'd need to either have a Drupal-only log
interface that has support for more stuff (which we said a few weeks ago
was OK) or just drop support for em-wrapped and pre-escaped values.
Based on my previous research into Drupal's current usage I don't
believe that would be all that onerous as @ ("you escape this") is
overwhelmingly the standard usage. If any of the other Drupalers on the
list wish to disagree with me though, please do. (Ping catch...)

In the short term, the big benefit of this PSR is that we could start
collecting logs from the various Symfony components we have now where
we're not passing in a log object at all, but really should be.
(Assuming Symfony moves to this interface, which I expect will happen.)

--Larry Garfield

[Paul Jones]

On Dec 12, 2012, at 3:04 PM, Larry Garfield wrote:

> On 12/12/2012 09:16 AM, Paul Jones wrote:
>>> In my opinion the ideal solution considering the modifiers would be something that reuses a single call to sprintf and adds named parameter support.
>> Well, we're not doing implementations per se, although writing example implementations can help us figure out what makes sense for a spec.
>
> Ordered placeholders are useless. Full stop. And if we're adding named placeholders to sprintf(), then we're not using sprintf() anymore so there's no programmatic benefit to modeling on sprintf().

Agreed.

>> In the LoggingIterface thread, Jordi has opined that he's ok with single brace separators. I think that's a move in the right direction.
>
> Another data point: Although I don't believe we'd discussed using it in the HTTP client thread, there is a draft IETF spec for URI patterns that uses {}, which is what Symfony and now Drupal are using. So IF we were to have placeholders in a future HTTP client thread, they'd probably be using {}. I think that's a mark in {}'s favor.

Also agreed.

>> There's also the Drupal guys, who use a single leading non-word character to indicate what kind of escaping needs to happen on the placeholder.
>>
>> With those in mind, I'd like to suggest the following as a general compromise:
>>
>> - Placeholder delimiters MUST use a single opening brace and a single closing brace, with no whitespace between the delimiters and the name
>>
>> - Placeholder names MUST be one or more word characters
>
> Should we qualify "word character" as "alphanumeric plus underscore?" (I don't know if that's exactly the same thing, not being a regex guru. Which probably means plenty of other people will interpret it that way, or possibly just alphabetic characters, which will lead to confusion. Let's be clearer on what characters are legal there.)

I think that is reasonable: /[A-Za-z0-9_]+/, yes?

>> - Placeholders MAY optionally have a single non-word non-whitespace character between the opening brace and the placeholder name; this character MAY be used by the implementor as information about how to treat the replacement string.

...

> While I appreciate you thinking about us :-), I don't think that will work. Remember, Solarium and Guzzle are the target audience here, not Drupal primarily.

...

So, in summary, you think having an optional leading character is not just suboptimal but actively negative in a lot of ways, and should be omitted. Is that correct?

> In the short term, the big benefit of this PSR is that we could start collecting logs from the various Symfony components we have now where we're not passing in a log object at all, but really should be. (Assuming Symfony moves to this interface, which I expect will happen.)

/me nods

So if I'm hearing you right, the acceptable rules would be:

- Placeholder delimiters MUST use a single opening brace and a single closing brace, with no whitespace between the delimiters and the name.

- Placeholder names MUST be composed only of the characters A-Z, a-z, 0-9, and underscores.

Is that correct? If so, do we want the name compliance to be MUST, or would we prefer SHOULD instead?

Thanks Larry.


-- pmj

[Alexandre Gaigalas]

On Wed, Dec 12, 2012 at 7:04 PM, Larry Garfield <la...@garfieldtech.com> wrote:

On 12/12/2012 09:16 AM, Paul Jones wrote:

Ordered placeholders are useless. Full stop.  And if we're adding named placeholders to sprintf(), then we're not using sprintf() anymore so there's no programmatic benefit to modeling on sprintf().

Of course there is benefit! sprintf isn't just a mere placeholder parser, it's also a formatter with %d and many other modifiers for string, which are pretty handy for logs and error messages. Extending the behavior of sprintf to accept named parameters would let us benefit from these modifiers as well.

--Gaigalas

[Beau Simensen]

On Dec 12, 2012, at 3:18 PM, Paul Jones <pmjo...@gmail.com> wrote:

> A-Z, a-z, 0-9, and underscores.

I think it would it make sense to expand this to include periods as
well. I have seen them used frequently to group and organize
placeholders.

[Florin Patan]

Since my first mention of sprintf() in this thread I haven't gotten the answer to a basic question so I'm asking, again, maybe someone will finally notice, why do we even need named parameters for logging?

It should be an implementation detail.

Granted, it's nice to know what you can use Zend Framework logger which implements said LoggerInterface in a Symfony2 project and log things from a third party library and if you switch any of them you won't break anything BUT, lets face it:

$this->logger->debug('this is a pretty message %s %d', array('string', 10));

Would be the same in every place.

If you implement named placeholder you'll get... 

$this->logger->debug('this is a pretty message %result% %value%', array('%result' => 'string', '%value%' => 10));

Internally you'll use sprintf in the first case strtr.

if you change the logging function from the first format to the second one, then for the user, the result would be bad, if you switch from the second to the first then there's no issue BUT again, why do you need named parameters and sprintf ones are not enough?

If it's because translation, then I'll state it again, translators would better need a context rather that a pretty named variable. If it's because you don't want to repeat something then think how often do you actually repeat a value in a log message or if you are doing more that 2 times then maybe you should reconsider your message format in the first place imho.

Keeping things simple should be in everyones head as look how far we've got with the talks from such a little thing.

Also remember that at the end of the day, we are talking about the LoggerInterface not LoggerComponent. Everyone should be allowed to implement their own things. Generalizing such a detail leads to so many problems... If in turn we'd take this to next level and make it a LoggerComponent then maybe this would be a valid talk point but I thought that FIG role is to standardize things rather that create the One framework :)

Also, extending sprintf() is a great idea, if you want to complicate things that are supposed to be simple! Why would you ever need to extend a function like sprintf, is beyond me, unless you're too Java minded :) (hope no one jumps when seeing this but it's the truth).

As a final word, we have strtr in PHP, why not just use it and send the parameters to the parameters array in a k/v pair and be done with? this way, everyone could implement what they want, it's flexible enough and, best of all, it is SIMPLE. Don't ever sacrifice simplicity over features that are used in edge cases is the lesson I've learned so far and it's proven to be preeety useful.

TLDR: This is an implementation detail, just use k/v pair to help usage of strtr and that's it, don't complicate stuff with problems that don't exist, exist in very very little edge-cases. Nobody expects to have 100% of every situation, nobody will ever do as we are humans, but the users of this are generally smart guys who can figure out how to extend basic things to do what they need to do in edge cases.

[Paul Jones]

On Dec 12, 2012, at 4:43 PM, Florin Patan wrote:

> TLDR: This is an implementation detail, just use k/v pair to help usage of strtr and that's it, don't complicate stuff with problems that don't exist, exist in very very little edge-cases.

Some folks (me! ;-) are going to want to throw a pre-existing associative array of data against the message, and won't want to modify that array before doing so. E.g.:

$context = ['foo' => 'zim', 'bar' => 'gir'];
$message = "foo the {foo} and bar the {bar}";
foreach ($context as $name => $text) {
$message = str_replace('{' . $name . '}', $text, $message);
}

With that, they don't need to add the delimiters around the array keys before passing it to the logger.

If folks want to use sequential parameters, they can do so like this (note that the replacement loop is identical):

$context = ['gir', 'foo'];
$message = "foo the {1} and bar the {0}";
foreach ($context as $name => $text) {
$message = str_replace('{' . $name . '}', $text, $message);
}

If one wants to use strtr(), one could do this (the replacement loop now builds a replacement array, and strtr() gets called afterwards).

$context = ['foo' => 'zim', 'bar' => 'gir'];
$message = "foo the {foo} and bar the {bar}";
$replace = [];
foreach ($context as $name => $text) {
$replace['{' . $name . '}'] = $text;
}
$message = strtr($message, $replace);

Hope that helps.


-- pmj

[Paul Jones]

Any objections?


-- pmj

[Florin Patan]

Great! Thanks!

But do you agree that this is an implementation detail?

And if so suggesting strtr for parameter replacing, like the autoloader from PSR-0 is a suggestion, it should be fine. The logging components which implements this interface could be extended to use whatever they want/need for their cases but are not mandatory.

If you want you can have something like this:

class PrettyLogger implements LoggerInterface {}

class PrettyerLogger implements LoggerInterface {

 setLogger(LoggerInterface $logger);

 function debug($msg, $params) {

  // take the params, prepare for strtr, call $this->logger->debug($msg, $formattedParams);

 }

}

$logger = (new PrettyerLogger())->setLogger(new PrettyLogger());

and that's it, everyone, well, besides those who don't like code like that.

Your pre-existing messages would be safe, implementations could be exchanged as you want and so on and like you've said, some, not all, want this to work the way you describe it ;)

Please what I'm writing as I'm also thinking in terms of performance and so on, i. e. no regex, no (heavy) pre-processing by default, if you want custom things, you (not you, pmjones, per se) alone should support any and all performance penalty hits by doing extra-work. 

And hey, it's a new thing we are talking about here, BC breaks will occur for everyone, the point is to have it as simple/extensible as possible and I see no other way to do it without over-complicating things.

Best regards.

[Paul Jones]

On Dec 12, 2012, at 5:27 PM, Florin Patan wrote:

> But do you agree that this is an implementation detail?

Yes, definitely.

> And if so suggesting strtr for parameter replacing, like the autoloader from PSR-0 is a suggestion, it should be fine.

Yes, those were suggestions/examples/references only, not implementations to be delivered.


-- pmj

[Florin Patan]

Great!

Now, let's ask Jordi to update the proposal with something like this:

The implementation of ::log() method MUST use strtr($message, $parameters) to format/display/do something to the message

And lets abandon all the side talk about delimiters and so on, we can all look into php,net/strtr to check for limitations, and help move the proposal to the voting stage and make it accepted.

Does it seem reasonable?

Best regards.

[Tim Otten]

On Dec 12, 2012, at 6:01 PM, Paul Jones wrote:

>
> On Dec 12, 2012, at 4:43 PM, Florin Patan wrote:
>
>> TLDR: This is an implementation detail, just use k/v pair to help usage of strtr and that's it, don't complicate stuff with problems that don't exist, exist in very very little edge-cases.
>
> Some folks (me! ;-) are going to want to throw a pre-existing associative array of data against the message, and won't want to modify that array before doing so. E.g.:
>
> $context = ['foo' => 'zim', 'bar' => 'gir'];
> $message = "foo the {foo} and bar the {bar}";
> foreach ($context as $name => $text) {
> $message = str_replace('{' . $name . '}', $text, $message);
> }

Aside: That algorithm (iterative str_replace) isn't safe. The values of $context should be regarded as untrusted, but this algorithm gives them special interpretation. A malicious user who can influence the values could do:

$context = ['foo' => 'zim{bar}', 'bar' =>'gir'];

which should output

"foo the zim{bar} and bar the gir"

but would actually output

"foo the zimgir and bar the gir"

I'm sure you can still get the intended result by using of the other substitution techniques (strtr() or preg_*()).

> With that, they don't need to add the delimiters around the array keys before passing it to the logger.
>
> If folks want to use sequential parameters, they can do so like this (note that the replacement loop is identical):
>
> $context = ['gir', 'foo'];
> $message = "foo the {1} and bar the {0}";
> foreach ($context as $name => $text) {
> $message = str_replace('{' . $name . '}', $text, $message);
> }
>
> If one wants to use strtr(), one could do this (the replacement loop now builds a replacement array, and strtr() gets called afterwards).
>
> $context = ['foo' => 'zim', 'bar' => 'gir'];
> $message = "foo the {foo} and bar the {bar}";
> $replace = [];
> foreach ($context as $name => $text) {
> $replace['{' . $name . '}'] = $text;
> }
> $message = strtr($message, $replace);
>
> Hope that helps.
>
>
> -- pmj
>

[Paul Jones]

On Dec 12, 2012, at 5:50 PM, Florin Patan wrote:

> Now, let's ask Jordi to update the proposal with something like this:
> The implementation of ::log() method MUST use strtr($message, $parameters) to format/display/do something to the message

No, I don't think that's wise. We don't specify implementations.



-- pmj

[Paul Jones]

On Dec 12, 2012, at 5:58 PM, Tim Otten wrote:

>
> On Dec 12, 2012, at 6:01 PM, Paul Jones wrote:
>
>>
>> On Dec 12, 2012, at 4:43 PM, Florin Patan wrote:
>>
>>> TLDR: This is an implementation detail, just use k/v pair to help usage of strtr and that's it, don't complicate stuff with problems that don't exist, exist in very very little edge-cases.
>>
>> Some folks (me! ;-) are going to want to throw a pre-existing associative array of data against the message, and won't want to modify that array before doing so. E.g.:
>>
>> $context = ['foo' => 'zim', 'bar' => 'gir'];
>> $message = "foo the {foo} and bar the {bar}";
>> foreach ($context as $name => $text) {
>> $message = str_replace('{' . $name . '}', $text, $message);
>> }
>
> Aside: That algorithm (iterative str_replace) isn't safe. The values of $context should be regarded as untrusted, but this algorithm gives them special interpretation. A malicious user who can influence the values could do:
>
> $context = ['foo' => 'zim{bar}', 'bar' =>'gir'];
>
> which should output
>
> "foo the zim{bar} and bar the gir"
>
> but would actually output
>
> "foo the zimgir and bar the gir"
>
> I'm sure you can still get the intended result by using of the other substitution techniques (strtr() or preg_*()).

Certainly; we'll need a better example, of course.


-- pmj

[Paul Jones]

On Dec 12, 2012, at 5:58 PM, Tim Otten wrote:

> Aside: That algorithm (iterative str_replace) isn't safe. The values of $context should be regarded as untrusted, but this algorithm gives them special interpretation. A malicious user who can influence the values could do:
>
> $context = ['foo' => 'zim{bar}', 'bar' =>'gir'];
>
> which should output
>
> "foo the zim{bar} and bar the gir"
>
> but would actually output
>
> "foo the zimgir and bar the gir"
>
> I'm sure you can still get the intended result by using of the other substitution techniques (strtr() or preg_*()).

So the example code should be closer to this:

$context = ['foo' => 'zim', 'bar' => 'gir'];
$message = "foo the {foo} and bar the {bar}";
$replace = [];

foreach ($context as $key => $val) {
$replace['{' . $key . '}'] = $val;
}
$message = strtr($message, $replace);

Yes?


-- pmj

[Tim Otten]

On Dec 12, 2012, at 7:24 PM, Paul Jones wrote:

> So the example code should be closer to this:
>
> $context = ['foo' => 'zim', 'bar' => 'gir'];
> $message = "foo the {foo} and bar the {bar}";
> $replace = [];
> foreach ($context as $key => $val) {
> $replace['{' . $key . '}'] = $val;
> }
> $message = strtr($message, $replace);
>
> Yes?
>

Yup, that's a safe algorithm.

[Hari K T]

Hey @pmj, 

I appreciate your skills to throw examples quickly and yes, your patience.

Hari K T
M: +91-9388758821 | W: http://harikt.com/blog

[Tim Otten]

On Dec 12, 2012, at 6:27 PM, Florin Patan wrote:

Great! Thanks!

But do you agree that this is an implementation detail?

Depends on what "this" refers to. If "this" means "using strtr vs str_replace vs preg_*", then yes, it's an implementation detail. If "this" means "%foo% vs {foo} vs %5d", then no, it's part of the interface. It affects interoperability.

 https://groups.google.com/forum/#!msg/php-fig/yL_wDyiT6GY/9oSHoolgWJMJ

And if so suggesting strtr for parameter replacing, like the autoloader from PSR-0 is a suggestion, it should be fine. The logging components which implements this interface could be extended to use whatever they want/need for their cases but are not mandatory.

If you want you can have something like this:

class PrettyLogger implements LoggerInterface {}

class PrettyerLogger implements LoggerInterface {

 setLogger(LoggerInterface $logger);

 function debug($msg, $params) {

  // take the params, prepare for strtr, call $this->logger->debug($msg, $formattedParams);

 }

}

$logger = (new PrettyerLogger())->setLogger(new PrettyLogger());

If a downstream library developer wants the functional benefits of consuming the LoggerInterface but dislikes the method signatures or input formats, then he can certainly create his own private logging API that wraps around the standard API. (I don't think it's a good idea, but it needn't affect the standardization process -- too each his own.)

In that spirit, your example can be OK -- BUT it needs some change. It's misleading to say that "PrettyerLogger implements LoggerInterface". Granted, PHP's type-checking system will allow it, but PrettyerLogger doesn't really follow the same contract as LoggerInterface (because it expects input-strings and input-arrays to be written with a different format). If someone took an instance of PrettyerLogger and injected it into a class which expected "LoggerInterface", then PHP would not raise an error -- but the result would be buggy, and the bug would go undiscovered until someone tried to read through the logs.

Some ways you could change it:

1) Omit the phrase "implements LoggerInterface". Then the type-checker will do its job -- it will prevent developers from making bugs by mixing incompatible objects.

2) Add new methods with different names instead of overriding methods. For example, you could add "sugaryDebug($message, ...varargs...)" which accepts a different notation and does any necessary translation between your notation and the standard notation. Then the standard-compliant methods will still be accessible, and your preferred sugary methods will also work.

Those would be safe. Personally, I wouldn't do either -- the overhead (documentation, configuration, maintenance, etc) would outweigh the gain of the syntax sugar. But... "to each his own". ;)

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/php-fig/-/Ni-Q4B9eakYJ.

[Larry Garfield]

No, it does not seem reasonable in the slightest.

The context array is intended to carry more than just string replacement values, thus making it an ordered array rather than associative is a non-starter.  The spec already says, for instance, that an exception must be passed with the key value of "exception".  Really, I do not understand the sudden obsession with less-functional, less self-documenting syntax. 

Dictating that an implementation must use strtr() is a complete non-starter; for one thing, many implementations won't even do replacement immediately but store the message and context array separately in a database or similar to then recombine on display... which is exactly why we need placeholders rather than just a raw string.

--Larry Garfield

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/php-fig/-/YmUfoHHdnJEJ.

[Larry Garfield]

On 12/12/2012 03:18 PM, Paul Jones wrote:
>>> There's also the Drupal guys, who use a single leading non-word character to indicate what kind of escaping needs to happen on the placeholder.
>>>
>>> With those in mind, I'd like to suggest the following as a general compromise:
>>>
>>> - Placeholder delimiters MUST use a single opening brace and a single closing brace, with no whitespace between the delimiters and the name
>>>
>>> - Placeholder names MUST be one or more word characters
>> Should we qualify "word character" as "alphanumeric plus underscore?" (I don't know if that's exactly the same thing, not being a regex guru. Which probably means plenty of other people will interpret it that way, or possibly just alphabetic characters, which will lead to confusion. Let's be clearer on what characters are legal there.)
> I think that is reasonable: /[A-Za-z0-9_]+/, yes?

Someone else suggested including . there. I'm fine with that, as long
as the list is clearly defined and not referenced on something
potentially vague like "word characters". (I'm sure someone will say
that is clearly defined, but only for people with strong regex-fu. I'm
certain that will lead to someone making an implementation that chokes
on something it nominally shouldn't or vice versa.)

>
>>> - Placeholders MAY optionally have a single non-word non-whitespace character between the opening brace and the placeholder name; this character MAY be used by the implementor as information about how to treat the replacement string.
> ...
>> While I appreciate you thinking about us :-), I don't think that will work. Remember, Solarium and Guzzle are the target audience here, not Drupal primarily.
> ...
>
> So, in summary, you think having an optional leading character is not just suboptimal but actively negative in a lot of ways, and should be omitted. Is that correct?

Having undefined leading characters would be harmful for
interoperability. If we want to have any such modifiers, they should be
explicitly defined and universal, or else they would harm interoperability.

I would be open to a statement like "additional modifier characters on
the placeholder value are reserved for future PSR use." Or something
like that. That signals that people should *not* start throwing in @ or
$ or whatever as modifiers, because we may want to revisit that in a
future PSR after this spec gets some field testing.

>
>> In the short term, the big benefit of this PSR is that we could start collecting logs from the various Symfony components we have now where we're not passing in a log object at all, but really should be. (Assuming Symfony moves to this interface, which I expect will happen.)
> /me nods
>
> So if I'm hearing you right, the acceptable rules would be:
>
> - Placeholder delimiters MUST use a single opening brace and a single closing brace, with no whitespace between the delimiters and the name.
>
> - Placeholder names MUST be composed only of the characters A-Z, a-z, 0-9, and underscores.
>
> Is that correct? If so, do we want the name compliance to be MUST, or would we prefer SHOULD instead?
>
> Thanks Larry.
>
>
> -- pmj

"SHOULD" implies "but you can't rely on it", which means that smart
implementers will have to treat it like a MUST". So if we say "it
should be alphanumeric", that means "but it may not be", so implementers
will have to deal with non-alphanumerics anyway to be safe, which means
the restriction is not meaningful in the first place.

Note: I'm probably showing my Anglo-centrism by referring to strict
Latin alphanumeric characters only here; I have no clue what the
implications of allowing accented characters as "alphanumeric" would be,
or what non-latin character sets would do here if someone were writing a
Chinese-only or Arabic-only program. I don't know if we want to deal
with that; I'm totally fine with ignoring that problem for now on the
assumption that everyone in the universe supports the same ASCII 127
characters and virtually all programmers in the world can at least read
it, but I mention it out of completeness.

--Larry Garfield

[Florin Patan]

Hi Larry, thanks for your input.

I'm not disagreeing with the idea of having placeholders, be them named or otherwise. I just don't think that the placeholder delimiter discussion helps us in this context.

True, maybe the ::log() function wasn't the perfect place to mention, give the consensus that logs can be stored in intermediary systems before them being used/displayed in a formatted manner, which I agree that's a common practice. Maybe the ::read/render/view/display/process/store/dumpLog() function would be a better for readability as a means to implement this.

But by forcing the placeholders to be in a certain type, we definitely define how things should be done/work/implemented which is the same as telling everyone that they should support k/v pairs for parameters and then use strtr/preg_replace/own compatible way to format the message when displaying it. This way, there won't be any limitations with regards of how things are separated and so on, implementations would just need to make sure that any and all occurrences of __key__ from parameter array will be matched and replaced with __value__ from the same array in the message. This way, if I decide to use ##placeholder## as key, I won't be constrained by the implementation which doesn't cover for the ## start delimiter and ## end delimiter.

Imho, this is in no way less functional/self-documenting/clear that any other proposal and like someone from the main thread suggested, there's always room for version 1.1 of this if we are not satisfied with the results in 1-2 years.

I hope that now I've made myself a bit more clear in regards to this.

Best regards.

[FGM at GMail]

Regarding the regexp, I dont think /[A-Za-z0-9]+/ is appropriate:

- it misses non-ASCII characters, which have been legit for some time
in PHP for symbols (but maybe we don't want them)
- more importantly, it accepts symbols starting by digits, which is
probably not good.

Rather something like /^([A-Za-z_])(\w+)$/
or possibly better /^([:alnum])(\w+)$/

2012/12/12 Paul Jones <pmjo...@gmail.com>:

> --
> You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
> To post to this group, send email to php...@googlegroups.com.
> To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.

[Evert Pot]

On Dec 13, 2012, at 12:19 PM, FGM at GMail <fgma...@gmail.com> wrote:

> Regarding the regexp, I dont think /[A-Za-z0-9]+/ is appropriate:
>
> - it misses non-ASCII characters, which have been legit for some time
> in PHP for symbols (but maybe we don't want them)
> - more importantly, it accepts symbols starting by digits, which is
> probably not good.
>
> Rather something like /^([A-Za-z_])(\w+)$/
> or possibly better /^([:alnum])(\w+)$/

Out of curiosity, what's the motivation for not allowing any unicode sequence (except the placeholder closing char)?

While I would personally not want to advocate non-ascii characters, I'd also certainly wouldn't want to prevent people from doing so, if they desire.

Evert

[Johannes Schmitt]

Do you propose that implementations enforce these placeholder keys? This might seriously impact performance of implementations, and anyway what does it solve?

Also regarding escaping, this is completely pointless unless we also add restrictions on how the message data itself has to look like. At the moment neither the message, nor the placeholder values can be considered safe in any way. The meaning "you escaped this", "we escape this" is flawed (at least in the context of logging). It is impossible to say "you escaped this" as it is impossible to know in which ways the resulting data is being used (some might display it in html, other use it in SQL, etc.). So, you cannot possibly escape anything before passing it to the logger. Such a meaning might work in other places, but for this interface it simply does not work. As such, I also think that it is pointless to work on a general placeholder concept, all we need is some basic placeholder support to avoid the sprintf() calls that people currently have to make in their libraries; nothing more, nothing less.

If you have concerns, why the current proposal does not address this, I'd be happy to hear them, but please let's not go overboard, and focus on real-world logging needs.

I'm also happy to discuss a proposal a few days longer that really does not matter, but I'm a bit tired to discuss the same things, arguments, ideas that we have already discussed a few weeks ago. Certainly, that is not a single person's fault, but mostly a result of the workflow that we currently have as we have no place where we record the concerns/ideas and how they have been addressed, and reading through all emails that have been sent in all the different threads is simply too impractical. So, we probably need to re-think this, but please let's keep that in a separate thread :)

Thanks,
Johannes

[FGM at GMail]

I think I may not have been explicit enough:

- I don't think we need any form of validation on placeholders,
precisely because of the cost and developer-controlled properties of
these placeholders: this is not user-generated content
- if we want it however, and decide on a regex, I think either of the
changes I suggested would be more appropriate than a plain \w+, for
the reasons outlined.

2012/12/13 Johannes Schmitt <schmi...@gmail.com>:

[Jason Judge]

On Thursday, 13 December 2012 08:05:32 UTC, Larry Garfield wrote:

No, it does not seem reasonable in the slightest.

The context array is intended to carry more than just string replacement values, thus making it an ordered array rather than associative is a non-starter.  The spec already says, for instance, that an exception must be passed with the key value of "exception". 

So there is a mix of developer-defined placeholder names, and reserved words, in the context array? Are there procedures in place to prevent clashes when the reserved word list gets extended?

-- Jason

[Jordi Boggiano]

It's not reserved, it just says if you want to pass an exception, pass
it in the exception key. See: 1.5 at
https://github.com/Seldaek/fig-standards/blob/logger-interface/proposed/logger-interface.md

It's just trying to set a best practice so that things work better
together, but it does not enforce anything.

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

[Jordi Boggiano]

On 13.12.2012 13:26, FGM at GMail wrote:
> I think I may not have been explicit enough:
>
> - I don't think we need any form of validation on placeholders,
> precisely because of the cost and developer-controlled properties of
> these placeholders: this is not user-generated content
> - if we want it however, and decide on a regex, I think either of the
> changes I suggested would be more appropriate than a plain \w+, for
> the reasons outlined.
>
> 2012/12/13 Johannes Schmitt <schmi...@gmail.com>:
>> Do you propose that implementations enforce these placeholder keys? This
>> might seriously impact performance of implementations, and anyway what does
>> it solve?

I agree with both. Actually yesterday I added this to the spec:

> Users SHOULD use placeholder keys that are alpha-numeric
> ([a-zA-Z0-9]). The use of other characters is reserved for possible
> extensions of the placeholders formatting.

That says keep to alpha-numeric chars or violate the spec at your own
risk. That way implementations can remain simple/lean, and people should
not complain if we add special characters in two years.

[Jason Judge]

I was reading too much into it, and understand better now. If an exception object is passed into the context array then it MUST be against the "exception" key. However, the opposite does not apply: the exception key can be used to carry ANY other types of data. It is up to the consumer to check what it contains before using it. Proposal just says where the exception will be IF it is supplied.

Thanks,

-- Jason

[Moisa Teodor]

Just a thought: if the context contains an exception, I wonder if we really need to look for it in a certain key. If the context needs to be looped through anyway. Something like:

    // inside the context loop
    if (is_object($element) && ($element instanceof \Exception)) {

        // we have the exception and do something with it

    }

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To post to this group, send email to php...@googlegroups.com.
To unsubscribe from this group, send email to php-fig+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msg/php-fig/-/HeuNPkWCVMcJ.

For more options, visit https://groups.google.com/groups/opt_out.
 
 

--
Doru Moisa
web: http;//moisadoru.wordpress.com
tel: +40 720 861 922
Bucharest, Romania

[Jordi Boggiano]

On 13.12.2012 16:20, Moisa Teodor wrote:
> Just a thought: if the context contains an exception, I wonder if we
> really need to look for it in a certain key. If the context needs to be
> looped through anyway. Something like:
>
> // inside the context loop
> if (is_object($element) && ($element instanceof \Exception)) {
> // we have the exception and do something with it
> }

As with any change proposal, please think about what the benefits are
and explain them together with the change. If you can't find any, it's
probably not worth debating over. (I don't mean to pick on you, it's a
general remark)

The fact that things have to be looped over is an implementation detail,
some might loop at various places. Some implementations might look for
an exception anywhere to do fancier things if they want. The spec just
tries to nudge people towards using the same thing, but if they don't
it's still ok. It's just pointing out a sort of gentlemen's agreement
that will enable some features without resorting to over-specification.

[Larry Garfield]

We've been over this:

Consider what not having a standard delimiter would mean for implementors:

I work on Drupal. Drupal 8 as of right now includes 3rd party libraries
from Symfony, Guzzle, Assetic, Doctrine, and Twig. (Yes, that's a
massive change from earlier versions. Woo!) Let's suppose all of those
libraries adopts LoggingInterface, which means we can bridge all of
those libraries to our existing logging system by creating a
ForeignLogger implements LogginInterface object and passing it into
objects of all of those libraries. Huzzah!

But now, suppose the Symfony libraries use {foo}, Guzzle uses #foo#,
Assetic uses %foo, Doctrine uses <foo>, and Twig uses its own {{foo}}.

That means one of two things has to happen:

1) Drupal now needs to make five different logging objects to pass into
different libraries, and then when we log the message to the database
for later display we have to track which set of placeholders each one is
using. So on display, we have to load the message, and the
placeholders, and the delimiters for that particular message, and build
up a regex specific for each message depending on the delimiters that
message is using.

2) Symfony, Guzzle, etc. need to specify, either in pre/post methods or
as part of the log call itself, what delimiters its using. Then
Drupal's ForeignLogger implementation will need to either store the
delimiters itself (and then repeat the problems in point 1) or normalize
the placeholders with a similar regex from point one to some internal
format. Same problem, different place.

Both of those are bad situations, and raise the complexity bar high
enough that a lot of people will simply say "meh, not worth it".
Frankly, so would I.

We've already established the need for placeholders. The above
establishes the need for a single delimiter. It looks like {} is the
leading delimiter right now. Can we stop talking about delimiters? :-)

--Larry Garfield

>> <javascript:>.

>> To unsubscribe from this group, send email to

>> php-fig+u...@googlegroups.com <javascript:>.

>> To view this discussion on the web visit
>> https://groups.google.com/d/msg/php-fig/-/YmUfoHHdnJEJ.
>> For more options, visit https://groups.google.com/groups/opt_out.
>>
>>
>
> --
> You received this message because you are subscribed to the Google
> Groups "PHP Framework Interoperability Group" group.
> To post to this group, send email to php...@googlegroups.com.
> To unsubscribe from this group, send email to
> php-fig+u...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/php-fig/-/8rUa-BCvAmAJ.

[Paul Jones]

On Dec 13, 2012, at 10:51 AM, Larry Garfield wrote:

> We've already established the need for placeholders. The above establishes the need for a single delimiter. It looks like {} is the leading delimiter right now. Can we stop talking about delimiters? :-)

At this point, I think so, yes. :-) I'll summarize the current position from this thread and submit it to Jordi for his approval.


-- pmj

[Paul Jones]

Guys, look this over as the draft replacement.

* * *

Replace the existing text:

- The message MAY contain placeholders in `{variable}` format (e.g. `{foo}`).
Those placeholders MAY be replaced by their corresponding value in the
context array. See for example:

```php
$logger = new AcmeLogger();
$username = 'Bob';

// MAY produce "User Bob created" or store
// "User {username} created" + a context array for later use
$logger->info('User {username} created', array('username' => $username));
```

Implementors MAY use placeholders to implement various escaping strategies
and translate logs for display. As such their use is very much encouraged.
Users SHOULD NOT pre-escape values however since they can not know in which
context the data will be displayed.

There is no support for deep replacement of placeholders. Placeholder names
match directly to the keys of the context array. Here is a sample
implementation to make it clear:

```php
$message = 'User {username} created';
$context = array('username' => $username);

foreach ($context as $key => $val) {

$message = str_replace('{'.$key.'}', $val, $message);
}
```

Users SHOULD use placeholder keys that are alpha-numeric (`[a-zA-Z0-9]`).
The use of other characters is reserved for possible extensions of the
placeholders formatting.

With the following new text:

- The message MAY contain placeholders which implementors MAY replace with
values from the context array.

Placeholder names MUST correspond to keys in the context array.

Placeholder names SHOULD be composed only of the characters A-Z, a-z,
0-9, underscore, and period. The use of other characters is reserved for
future modifications of the placeholders specification.

Placeholder names MUST be delimited with a single opening brace and a
single closing brace. There MUST NOT be any whitespace between the
delimiters and the placeholder name.

The following is an example implementation of placeholder interpolation
provided for reference purposes only:

```php
/**
* Interpolates context values into the message placeholders.
*/
function interpolate($message, array $context = array())
{
// build a replacement array with braces around the context keys
$replace = array();

foreach ($context as $key => $val) {
$replace['{' . $key . '}'] = $val;
}

// interpolate replacement values into the the message and return
return strtr($message, $replace);
}

// a message with brace-delimited placeholder names
$message = "User '{username}' created";

// a context array of placeholder names => replacement values
$context = array('username' => 'bolivar');

// echoes "Username 'bolivar' created"
echo interpolate($message, $context);
```

* * *


-- pmj

[Jeremy Lindblom]

Well, I wasn't convinced that we needed to worry about placeholders at all for the proposal, so I wasn't paying much attention to this thread. After reading through everything and seeing Larry's comments this morning, I'm in support of Paul's most recent modifications/additions to the draft. Let's wrap this up.

--
Jeremy Lindblom
PHP Software Development Engineer & Web Application Developer
Amazon Web Services – AWS SDK for PHP
@jeremeamia • https://github.com/jeremeamia

-- pmj

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.

[Chuck Burgess]

Looks good to me...

CRB
about.me/ashnazg

[Paul Jones]

On Dec 13, 2012, at 11:51 AM, Chuck Burgess wrote:

> Looks good to me...

Right on guys. I'll send to Jordi in the other thread.


-- pmj

[Jordi Boggiano]

On 13.12.2012 18:38, Paul Jones wrote:
> Guys, look this over as the draft replacement.

It's essentially rewording and adding _/. to the placeholders, so I am
obviously fine with it.

I would like to keep this block though:

> Implementors MAY use placeholders to implement various escaping
> strategies and translate logs for display. As such their use is very
> much encouraged. Users SHOULD NOT pre-escape values however since
> they can not know in which context the data will be displayed.

It shows why placeholders can be useful to others even if one does not
think it's useful to them directly.

[Paul Jones]

On Dec 13, 2012, at 12:12 PM, Jordi Boggiano wrote:

> On 13.12.2012 18:38, Paul Jones wrote:
>> Guys, look this over as the draft replacement.
>
> It's essentially rewording and adding _/. to the placeholders, so I am
> obviously fine with it.
>
> I would like to keep this block though:
>
>> Implementors MAY use placeholders to implement various escaping
>> strategies and translate logs for display. As such their use is very
>> much encouraged. Users SHOULD NOT pre-escape values however since
>> they can not know in which context the data will be displayed.

No objections from me, although the phrase "As such their use is very much encouraged" is ambiguous. I think it can be safely removed without distorting the thrust of the paragraph.


-- pmj

[Moisa Teodor]

Hi,

Sorry if it was unclear, but it was not a change proposal, more like a point of view and a question regarding the need to put the exception in some predefined context key. I did not mean to offend anyone, and I am sorry if I did. 

On Thu, Dec 13, 2012 at 5:27 PM, Jordi Boggiano <j.bog...@seld.be> wrote:

As with any change proposal, please think about what the benefits are
and explain them together with the change. If you can't find any, it's
probably not worth debating over. (I don't mean to pick on you, it's a
general remark)

The fact that things have to be looped over is an implementation detail,
some might loop at various places. Some implementations might look for
an exception anywhere to do fancier things if they want. The spec just
tries to nudge people towards using the same thing, but if they don't
it's still ok. It's just pointing out a sort of gentlemen's agreement
that will enable some features without resorting to over-specification.

[Jordi Boggiano]

Heya,

> Sorry if it was unclear, but it was not a change proposal, more like a
> point of view and a question regarding the need to put the exception in
> some predefined context key. I did not mean to offend anyone, and I am
> sorry if I did.

No offense at all no. It's just hard to know what people mean or want
sometimes, and there are lots of emails flying around, so it gets really
exhausting. I was just trying to explain how to get your point across in
a more helpful way (from the point of view of someone trying to moderate
the discussion anyway).

In any case it's now in voting phase, so I hope everyone is reasonably
happy with the status quo :)

[Ivan Enderlin]

On 11/12/12 23:28, Paul Jones wrote:

Hi all,

Hi,

What delimiters to use?
-----------------------

These are the ones I know are used by various projects, members or otherwise, whether for logging interpolation or for other string substitutions (please feel free to add to this list *from existing projects*).

    Drupal:
        %foo
        @foo
        !foo

    ext/intl and possibly others:
        {foo}
    
    Lithium, Solar/Aura:
        {:foo}

    Twig, Mustache, et al.:
        {{foo}}     (no spaces allowed)
        {{ foo }}   (zero or more spaces allowed)
    
    Suggested by Jordi:
        %foo%

Zsh uses the “zFormat” format, which is basically: (:subject[:options]*:). Thus we can have aVariable without option: (:aVariable:), or we can have aVariable with some options: (:aVariable:opt1:opt2:), or if options are represented by one character: (:aVariable:xyz:).

Hoa uses also this formalism and adds constants (prefixed by _). If we would like to recursively call a key in a set of parameter, we can prefix aVariable by a %. Let me show you an example:

     Let keywords $k and parameters $p:

       $k = array(
           'foo'      => 'bar',
           'car'      => 'DeLoReAN',
           'power'    => 2.21,
           'answerTo' => 'life_universe_everything_else',
           'answerIs' => 42,
           'hello'    => 'wor.l.d'
       );

       $p = array(
           'plpl'        => '(:foo:U:)',
           'foo'         => 'ar(:%plpl:)',
           'favoriteCar' => 'A (:car:l:)!',
           'truth'       => 'To (:answerTo:ls/_/ /U:) is (:answerIs:).',
           'file'        => '/a/file/(:_Ymd:)/(:hello:trr:).(:power:e:)',
           'recursion'   => 'oof(:%foo:s#ar#az:)'
       );

     Then, after applying the zFormat, we get:

       • plpl:        'Bar'
                       put the first letter in uppercase;

       • foo:         'arBar'
                       call the parameter plpl;

       • favoriteCar: 'A delorean!'
                       all is in lowercase;

       • truth:       'To Life universe everything else is 42'
                       all is in owercase, then replace underscores by spaces, and
                       finally put the first letter in uppercase; and no
                       transformation for 42;

       • file:        '/a/file/20090505/wor.21'
                       get date constants, then
                       get the tail of the path and remove extension twice,
                       and add the extension of power;

       • recursion:   'oofarBaz'
                       get 'arbar' first, and then, replace the
                       suffix 'ar' by 'az'.

This format is maybe a bit too complex for our current goal but it can be inspiring.
Note: it is very modular and powerful enought to fit 99% of use-cases (which are parameters.

Thoughts?

-- 
Ivan Enderlin
Developer of Hoa
http://hoa.42/ or http://hoa-project.net/

PhD. student at DISC/Femto-ST (Vesontio) and INRIA (Cassis)
http://disc.univ-fcomte.fr/ and http://www.inria.fr/

Member of HTML and WebApps Working Group of W3C
http://w3.org/