Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline mode #157

Merged
merged 4 commits into from
Jun 22, 2023
Merged

Inline mode #157

merged 4 commits into from
Jun 22, 2023

Conversation

fetsh-edu
Copy link
Contributor

  1. Fixes link in README
  2. Fixes inlineQueryChatType to support sender type (Inline mode is broken #156)
  3. Removes Null values for Json while adding Json fields with addJsonFields function (Inline mode is broken #156). Although it doesn't seem like a good idea :) Maybe it's better to handle Maybe somewhere closer to toJSON. For each field independently, somewhere along the lines of [ "normal_field" .= normal_field ] ++ maybe [] (\value -> ["maybe_field" .= value]) maybe_field

@fetsh-edu
Copy link
Contributor Author

Looks more like a proof of concept than a real pull request :) But with this changes Inline mode works.

@fizruk fizruk requested a review from swamp-agr May 27, 2023 17:39
@@ -34,13 +34,28 @@ data InlineQuery = InlineQuery
, inlineQueryLocation :: Maybe Location -- ^ For bots that require user location, sender location
, inlineQueryQuery :: Text -- ^ Text of the query, up to 256 characters
, inlineQueryOffset :: Text -- ^ Offset of the results to be returned, can be controlled by bot
, inlineQueryChatType :: Maybe ChatType -- ^ Type of the chat, from which the inline query was sent. Can be either “sender” for a private chat with the inline query sender, “private”, “group”, “supergroup”, or “channel”. The chat type should be always known for requests sent from official clients and most third-party clients, unless the request was sent from a secret chat.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please extend ChatType instead of introducing InlineChatType?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought about it, my reasons were: ChatType which is in Chat can not be sender:

Type of chat, can be either “private”, “group”, “supergroup” or “channel”

So if someone is pattern matching against this field now he would get non-exhaustive patterns, and will also have to handle this non-existent case. This way i was trying to minimize damage to existing code which uses the library.

But, yes, I can also see the reasons against it.

Copy link
Collaborator

@swamp-agr swamp-agr Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, as far as I see there are no usages of ChatType. Could you please remove it in favour of InlineChatType then?

@@ -1,7 +1,7 @@
cabal-version: 1.12

name: telegram-bot-api
version: 6.7
version: 6.7.1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please undo this change? It breaks my maintenance flow.

@fetsh-edu
Copy link
Contributor Author

@swamp-agr Haven't seen your last comment, so I've removed InlineChatType in favor of extended ChatType,

  • Undid version change,
  • And fixed an example link in the second README file.

@swamp-agr
Copy link
Collaborator

Thank you! Please expect release tomorrow.

@swamp-agr swamp-agr merged commit 5fa96c4 into fizruk:master Jun 22, 2023
@swamp-agr
Copy link
Collaborator

Please find the release on Hackage https://hackage.haskell.org/package/telegram-bot-api-6.7.1 (Haddock will be ready in a few days)

@swamp-agr swamp-agr mentioned this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants