botman / driver-telegram

BotMan Telegram Driver
MIT License
87 stars 75 forks source link

add parse mode #28

Closed dmamontov closed 3 years ago

dmamontov commented 6 years ago

Docs: https://core.telegram.org/bots/api#markdown-style

codecov[bot] commented 6 years ago

Codecov Report

Merging #28 into master will decrease coverage by 0.14%. The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #28      +/-   ##
============================================
- Coverage      77.5%   77.35%   -0.15%     
- Complexity      115      117       +2     
============================================
  Files            11       11              
  Lines           369      371       +2     
============================================
+ Hits            286      287       +1     
- Misses           83       84       +1
Impacted Files Coverage Δ Complexity Δ
src/TelegramDriver.php 92.9% <50%> (-0.57%) 46 <0> (+2)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a4c86fc...2e7a2ac. Read the comment docs.

mpociot commented 6 years ago

Hi! Thank you for the PR.

Isn't it better to configure this in the outgoing message, rather than globally through configuration?

For example, this is already possible and would break with this PR, as the global configuration would override the custom setting:

$bot->reply('This is **markdown**', [
    'parse_mode' => 'Markdown'
]);

We could add this to the global configuration and apply it in the array_merge_recursive, so that people could still override it.

dmamontov commented 6 years ago

In fact, it is not always convenient to specify locally. Because why repeat the same thing, if for example it is a bot for polls, and there are more than 100 different actions in it! Very messy code becomes. Therefore, in my project, I decided to do it globally!

mpociot commented 6 years ago

Yeah you're right. In this case, it will definitely make sense to do this globally.

Can you adjust your code, so that the local configuration could still override the global configuration?

christophrumpel commented 6 years ago

@dmamontov did you stop working on that? Would also need that :-)

dmamontov commented 6 years ago

Today I'll add an update! He has already done to himself for a long time. missed on github)

anthonyaxenov commented 4 years ago

@dmamontov @luizjr From one hand I am surprised about parse_mode is still not implemented in master (omg). From another hand, I have two things to say since this PR a lil bit diverge with reality.

1) Value of parse_mode must be in lowercase. Yes, this is not mentioned in official docs, but in fact when you pass value in another case the message won't be sent. (UPD: it's ok, my mistake)

2) Today we can pass one of three values: html, markdown (legacy) and markdownv2 (richer). Third value is not implemented, first and second ones are implemented wrong.

Small snippet to check:

$botman->hears('Button', function (\BotMan\BotMan\BotMan $bot) {
    $bot->ask('You pressed: *Button*', function (Answer $answer) {
        $this->say($answer->getText());
    }, array_merge([
        //'parse_mode' => 'HTmL'
        //'parse_mode' => 'MaRkDoWn'
        'parse_mode' => 'MARKDOWNV2'
    ], Keyboard::create()
               ->type(Keyboard::TYPE_KEYBOARD)
               ->addRow(KeyboardButton::create('Button'))
               ->toArray()));
});

Since there are no updates maybe later I can propose relevant improvements of 2e7a2ac (if I will not forget).