Add the ability to send markdown messages

[filipekiss]

Telegram has added an option to send markdown formatted messages passing Markdown as parse_mode when sending a message, as seen here. I couldn't seem to find how to use this option.

[jonnywilliamson]

Firstly Telegram are awful for creating breaking changes.

The original sendMessage Method they create was like this:

https://github.com/irazasyed/telegram-bot-sdk/blob/master/src/Api.php#L234-L257

public function sendMessage(
        $chat_id,
        $text,
        $disable_web_page_preview = false,
        $reply_to_message_id = null,
        $reply_markup = null
    ) 

Now they want to do this:

public function sendMessage(
        $chat_id,
        $text,
        $parse_mode,
        $disable_web_page_preview = false,
        $reply_to_message_id = null,
        $reply_markup = null
    ) 

If we add this feature as it stands it will break everyone's code (or those who had based their code on the order of parameters in the first example).

@irazasyed wasn't happy to do this as he had just pushed v1.0 only 12 hours before this change came out.

As it stands he hasn't made a decision to either go with v2.0 and include the new parameter - but run the rish of Telegram changing parameter order again willy nilly at the drop of a hat. Or do a completely different approach of only supplying an array to the method making parameter order a non issue.

Also, it does also say:

For the moment, only Telegram for Android supports this.

So it seems like a feature that isn't really all that well supported.

[filipekiss]

It works either for Android and for Desktop, as I just tested with my bot (Screenshot included). Also, @irazasyed could just add the parameter as the last option to prevent breaking anything, at least to give us some way to use that. parse_mode is an optional argument, so

public function sendMessage(
        $chat_id,
        $text,
        $disable_web_page_preview = false,
        $reply_to_message_id = null,
        $reply_markup = null,
        $parse_mode = null
    ) 

would work just fine without breaking changes. (I actually did this to try it out and I can make a pull-request if you guys think it's worth to give the users this option).

[jonnywilliamson]

Oh I already offered the PR with the parameter at the end, had the chat with him back on September 9th 2015!!

I'll leave it to @irazasyed to decide what he wants to do.

[filipekiss]

Ok, no problem 😄

I'll just extend the Api class to add the parameter for now :)

[irazasyed]

Hey guys,

So since we can see Telegram keeps pushing changes every now and then which is breaking this library and effecting us all with our releases due to them not having any proper release cycle or following any standards. I think its about time we conclude to one of the available options we have in hand. As @jonnywilliamson already said, We had this discussion few months ago as well because of this very same reason of Telegram making changes at random times and making things difficult for us.

Hence, We have 3 options (From based on your ideas as well as mine) and I'm open for your inputs (This applies to everyone including the ones who've not participated in this thread).

Following are the options that we can go with and their pros & cons:

1. We keep appending the params as and when they're added by Telegram.
   +Point: This is definitely easier for us to make changes and to keep up with Telegram's unpredicted random changes they make and for us to push them right away without having to worry about breaking anyones bot (This doesn't apply for all the cases).
   -Point: The library will still break if one of their params is removed from anywhere or they make the param that we appended as the required param (In which case, The developer is forced to either use the other params or add nulls/whatever only to add the required value to the param we just positioned at last). Not something we can rely on and i can see we might have to reposition all the params once again if this happens causing it to break again and probably another discussion out of frustration of all these changes breaking our bots/library. Besides these things, The Developer now has to educate himself about this library in depth and can't simply just get started right away after referring the official docs. The dev is now expected to go through the library docs to understand each and every params position and how everything works whereas he could've just started and assumed the params position based on official docs.
2. We simply switch to the array method where the dev is expected to pass the params as an array to the method and those can be anything. What we can also do is, Have a checker in place to throw an exception which would simply check if the minimum required params are present for that method or not. Ex: Chat ID and Message for sendMessage the library would need at least these params to make an API request properly.
   +Point: We no longer have to worry about Telegram's random param changes, Whether they add a new one or remove anyone or reposition wherever they like. The library will still continue to function properly and we wouldn't be breaking anyones bot. And the devs need not worry about learning library docs fully or keep referring it every now and then. As long as they refer the official docs and they wanna use some param, they would just pass it to their params array. And we would only have a major release only when its really a breaking change (Either due to Telegram's major changes or due to the library's internal changes -- Though we'll still try our best to maintain backwards compatibility).
   -Point: Building an array which as compare to the current method might not be something you wanna do, Since you now are expected to give the option name and value whereas now you simply supply a value to the method which then builds an array behind the scenes. Besides this, The current bot developers using this library would have to make major changes in their codes to adapt this major breaking change but it would be one time hopefully.
3. We keep pushing changes as and when they're made while keeping the same method we use (Without a major release) and just probably follow a proper release cycle for ourselves (Like maybe only once a month we push a major new version at a given date) and the devs can come back on that date or later to see what breaking changes have been made and accordingly update their codes if they want.
   +Point: We wouldn't be breaking any bots and keep the library updated & would be following a proper release cycle. Also, Any breaking changes within this period (before the major release is tagged) would all be covered (So no matter how many changes Telegram makes in this period, We'll be able to support them all) and any other changes would automatically go under next release.
   -Point: One new param is added, removed or repositioned (Or some change that could break the library, even though its a very small one from Telegram's side), The devs would have to wait until the next major release/tag of this library (Though they can still use master branch but that isn't recommended for obvious reasons).

So that's about it. I can only think of these options and their positive/negative points right now. If i think of any more, I'll post them again here. I'll let you guys pick one option from these as well as give your inputs and maybe more ideas if any.

Thanks!

[irazasyed]

We could also just keep doing major releases for every change that is breaking this library but that means, The devs would now have to keep updating their composer files and incrementing the major version number. While this is convenient for me, I'm sure it isn't something you wanna do that just to get that one param update. Besides this, There's a high chance we'll hit big version numbers in a very short period of time just to keep up with those simple param changes which technically are actually minor but for library it has become major. I see no point in following SemVer in such case TBH.

[jonnywilliamson]

Heres an odd ball idea.

How about using the change log date as our release number?

Here's what I mean: This is the current release/changelog:

Etc etc.

What if we said that our version numbers now match the release date? So instead of v1.0.1 we would have using the dates above:

v2015.10.08
v2015.09.18
v2015.09.07
v2015.08.29
v2015.08.15

So what if we don't have v3 or v4 and jump 2000 major points. It's only numbers - there's plenty more where they came from.

The ONE MAIN rule would be simple, the version you download from this repository will support exactly the relevant change-log from the Telegram website.

So if the version of the API here says 2016.01.22 but Telegram says the latest change was on 2016-Feb-02 then the developer knows that there may be features not implemented/or have been changed.

Bit of a crazy idea, I haven't ever seen it done...but any thoughts?

As for your worry about updating composer...well in this case you'd only have to do it once a year ie in composer ~2016

Otherwise I like idea 3.

[irazasyed]

Well, The idea is indeed crazy and I've honestly never seen this being done in any projects nor done it myself. My only concern is with the composer, How would it handle these changes/updates?

And what happens in case there is an internal change with the library that has got nothing to do with Telegram? For example it could be a major change, minor, patch/bugfixes, etc.

It's a bit odd to have that kinda versioning and not really a standard (We'd end up following Telegram devs, Since they don't seem to follow any standards, We too would be on their path which TBH i don't want to go their ways).

Lmk your thoughts on this.

Also, I'd vote for Option 2 with a combination of Option 3 (Have a proper release cycle, Any major changes internally or from Telegram would be pushed once a month only, I see this kinda release cycle being followed in a few projects).

[ihoru]

@irazasyed, you may create a poll (google sheets) and post link here to let us show what way of further development most preferable for library users (a kind of opinion statistic).
As for me, I like option 2 the most. In this case it would be possible to use features as fast as Telegram release them without many troubles.

[filipekiss]

@irazasyed The combination of Options 2 and 3 seems to be the one that may affect more users on a first update, but will make it easier to keep up-to-date with Telegram crazy changes and keep the library up-to-date. I don't see the need to use the dates as release versions, even tough I guess composer wouldn't have problems with it, but any changes made to the library that has nothing to do with Telegram API would add a layer of complexity to the version number (For example, version v2015.10.08-01 to add some library change but no changes from Telegram);

[filipekiss]

Just realised that the array options will also allow the parameter changes to work without the need to update the library.

Instead of having

public function sendMessage(
        $chat_id,
        $text,
        $disable_web_page_preview = false,
        $reply_to_message_id = null,
        $reply_markup = null
    ) {
        $params = compact('chat_id', 'text', 'disable_web_page_preview', 'reply_to_message_id', 'reply_markup');
        $response = $this->post('sendMessage', $params);

        return new Message($response->getDecodedBody());
    }

You'd have

public function sendMessage($params) {
        if (!is_array($params)) {
            //Throw some Exception
        }
        $response = $this->post('sendMessage', $params);

        return new Message($response->getDecodedBody());
    }

So when a new parameter is added all the developer has to do is add it to his array. He won't need to wait for a library update.

[irazasyed]

@ihoru Alright, Will do that soon.

@filipekiss When i say combination, I actually meant to have the major release tagging when there's actual major change from either Telegram's end or library itself (Not when the params are changed). All other minor changes would be pushed as and when they're made. The array method as you've already figured out would cut down our requirements of tagging the releases when Telegram makes these param changes and the library would be up to date in cases of these param changes. It'll give more control to the developer and at the same time resolve our problem with Telegram's crazy updates while following SemVer + Release Cycle.

[filipekiss]

@irazasyed Yes, I understand. I see no major problems with this approach except, as I said earlier, the first impact this would have on developers who need to change their code to match the new method signature. But still, the benefits seems to make up for that drawback, it would make the library stay closer to Telegram changes (at least when talking about parameters) and keep the versioning sane.

[irazasyed]

Yep! Indeed.

Here's the Poll Form: http://goo.gl/forms/qVIoMmtUpb please do your bit and submit your selection.

I'll keep this poll and issue open for a few days and then we'll take things forward from there. Thanks all.

[jonnywilliamson]

Ok, I know my idea was crazy, I just wanted us to have the highest version number in packagist....heheh

Anyway, I'm only in favour of number 2 IF we can also improve the doc blocks at the same time. It drives me mad having to refer to documentation for the names of parameters when I'm in the middle of coding. Really slows me down. Having it visible in the IDE or whatever is a real boost to productivity.

So I had a play around with what we could do and I came up with this using some features of docblock I hadn't seen before. Let's use the sendmessage method as an example.

Changing the docblock to this:

Allows full documentation when coding:

If I had this I would be happy with option 2. I think that looks clean and easy to understand.