[RFC|Accepted] Simple message API

[igorw]

Olympus has a BBCode parser that can be accessed by using the parse_message class. This class however has a kind of complex API and also includes other concepts such as attachments. Luckily there's the generate_text_for_storage, generate_text_for_display and decode_message functions which simplify this to a great extent (more info). But the API is still more complex than it should be.

It would be great to have a simple API for working with BBCode. This would make it a lot easier for MOD authors to include such functionality in their MODs. The new class would be called phpbb_message (working title) and act as a wrapper around the existing functions. It should use conventions where possible to reduce the effort required to use it.

Storing messages

First you have to set everything up. You can use allow_*() or allow() to set options.

Code: Select all

$text = request_var('text', '', true);
$message = new phpbb_message($text);

// these all default to true
$message->allow_bbcode(false);
$message->allow_urls(false);
$message->allow_smilies(false);

// or a shorter API
$message->allow_formatting(false, false, false); 

If your naming does not follow the convention (field name 'text' and no prefix), you will have to set those.

Code: Select all

// since your db fields may use a prefix, e.g. prefix_bbcode_uid, you may need to set it
$message->prefix('prefix_');

// if you want to use store(), you will have to specify which field to store the text in:
$message->text_field('text');

// you can also use:
$message->configure('text', 'prefix_'); 

To store the data, you can merge the options into an existing $sql_ary to be used for UPDATE or INSERT.

Code: Select all

$sql_ary = array(
  'some_field' => $some_field,
);

/*
this returns:
array(
  'prefix_text' => $text,
  'prefix_bbcode_uid' => $bbcode_uid,
  'prefix_bbcode_bitfield' => $bbcode_bitfield,
  'prefix_options' => $options,
)
*/
$sql_ary = array_merge($sql_ary, $message->store());

// shortcut (pass-by-reference)
$message->store_merge($sql_ary);

// in case you want to use prefix_enable_bbcode, prefix_enable_urls and
// prefix_enable_smilies instead of the prefix_bbcode_options bitfield,
// you'll have to use one of these:
$sql_ary = array_merge($sql_ary, $message->store(true));
$message->store_merge($sql_ary, true); 

And in case you want to read the attributes individually:

Code: Select all

// you can also access the attributes individually using
$message->text();
$message->options();
$message->allow_bbcode();
$message->allow_urls();
$message->allow_smilies(); 

Displaying messages

First it must be set up.

Code: Select all

// $row from db
$message = new phpbb_message($row);
$message->configure('text', 'prefix_');

// of course you can also use
$text = request_var('text', '', true);
$message = new phpbb_message($text); 

And displaying is as simple as can be.

Code: Select all

// diplay it
echo $message->display();

// or even (using toString)
echo $message; 

Decoding messages

Decoding messages means converting them as bbcode, for example to edit them. This is really as easy as displaying them:

Code: Select all

// $row from db
$message = new phpbb_message($row);
$message->configure('text', 'prefix_');

// convert to bbcode
echo $message->to_bbcode();

// alias
echo $message->decode(); 

Since this is new code, there is no concern with backwards compatibility. Other new features could also take advantage of this API.

Todo

• tracker ticket
• interface spec
• wiki docs
• implementation

Ticket: http://tracker.phpbb.com/browse/PHPBB3-9739

[naderman]

This was kind of the idea behind the formatted_text class I requested for 3.2 before: http://github.com/phpbb/phpbb3/blob/fea ... d_text.php

[imkingdavid]

/me likes. Looks great and a lot easier than the current system!

[DavidIQ]

Sounds really good. I would change this however:

Code: Select all

// or a shorter API
$message->allow(false, false, false); 

to maybe:

Code: Select all

// or a shorter API
$message->allow_formatting(false, false, false);  

Trying to make the purpose of the function more obvious.

[Dog Cow]

An array to set options which looks like this:

Code: Select all

$message->set_options(array('bbcode' => false, 'smilies' => false, 'urls' => false));

So I don't have to remember the order of arguments. Plus I can leave out some of those options if I wanted to accept defaults.

[igorw]

So I don't have to remember the order of arguments. Plus I can leave out some of those options if I wanted to accept defaults.

You could just use separate method calls for each in that case.

$message->allow_formatting(false, false, false);

I like that, will add to first post.

[igorw]

What do you guys think about the old implementation linked to by Nils?

What I like:

• Forces a standard column naming and format
• The naming itself "text", "meta", "flags"

What I don't like:

• The message class knows about the table name and the db connection

Before I make any other changes... any more thoughts on this?

[EXreaction]

Forces a standard column naming

Forcing the use of specific names just means it's not going to be good enough to use on everything and there will be conflicts if you have to join two or more tables.

Having a prefix option for the names would solve that.

It shouldn't do anything in the database unless you tell it to specifically, you're just adding an extra query on top of the other insert/update query that is needed.

[naderman]

Forces a standard column naming

Forcing the use of specific names just means it's not going to be good enough to use on everything and there will be conflicts if you have to join two or more tables.

Having a prefix option for the names would solve that.

That is exactly what it has. What he means by "standard column naming" is that the suffix is always the same. Of course you can use arbitrary prefixes. Otherwise you couldn't have multipled "formatted texts" per table.

It shouldn't do anything in the database unless you tell it to specifically, you're just adding an extra query on top of the other insert/update query that is needed.

I agree, however I believe that this kind of a class is only really useful when it also has convenience methods for working with the database. Either this can be split up into multiple classes or some functions simply need the database while others don't.

[igorw]

I agree, however I believe that this kind of a class is only really useful when it also has convenience methods for working with the database.

Would you consider the "array merge" approach convenient enough? It has the benefit of better separation of concerns.