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

Feature: 添加多消息段命令解析支持 #2419

Merged
merged 8 commits into from
Oct 18, 2023

Conversation

RainEggplant
Copy link
Contributor

@RainEggplant RainEggplant commented Oct 14, 2023

背景

采用 nonebot/adapter-telegram 作为 Telegram 的 adapter 时,带有命令的消息是分段的。例如 "/weather Beijing" 将会被分为 ["/weather", " Beijing"] (注意第二段包含空白符)。

现有 bug

现有的命令 parsing 的结果是

  • prefix[CMD_ARG_KEY] = " Beijing" (包含了空白符)
  • prefix[CMD_WHITESPACE_KEY] = None (无法识别空白符)

在这种场景下,on_commandforce_whitespace 参数也会失效。

修复

增加对于分段消息的处理。修复后的 parsing 结果是

  • prefix[CMD_ARG_KEY] = "Beijing" (删除了左侧空白符)
  • prefix[CMD_WHITESPACE_KEY] = " "

@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2023

🚀 Deployed on https://deploy-preview-2419--nonebot2.netlify.app

@github-actions github-actions bot temporarily deployed to pull request October 14, 2023 14:02 Inactive
@yanyongyu yanyongyu added the enhancement New feature or request label Oct 16, 2023
@yanyongyu yanyongyu changed the title 🐛 fix command parsing for segmented message (Telegram) Feature: 添加多消息段命令解析支持 Oct 16, 2023
@RainEggplant
Copy link
Contributor Author

似乎过不了一个测试,稍等我下午修一下,然后再增加一个类似 Telegram 多段消息的测试样例。

@RainEggplant RainEggplant marked this pull request as draft October 16, 2023 04:42
Copy link
Member

话说为什么不尝试先合并字符串呢

@yanyongyu
Copy link
Member

nb这里认为不同消息段哪怕is_text都是true,也认为是两个不相干的段落

@RainEggplant RainEggplant marked this pull request as ready for review October 16, 2023 06:09
@github-actions github-actions bot temporarily deployed to pull request October 16, 2023 06:13 Inactive
@RainEggplant
Copy link
Contributor Author

话说为什么不尝试先合并字符串呢

原来的代码就没有先合并再解析。刚刚接触 nonebot, 所以就尽量进行最少的修改了,免得有什么我不知道的情形会导致 bug。

nonebot/rule.py Outdated Show resolved Hide resolved
nonebot/rule.py Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request October 18, 2023 06:22 Inactive
@yanyongyu
Copy link
Member

试一下现在的解析有没有问题?我这没有验证过

@RainEggplant
Copy link
Contributor Author

试一下现在的解析有没有问题?我这没有验证过

试了下,没有问题。

tests/test_rule.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #2419 (d840a7c) into master (6559b2f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2419   +/-   ##
=======================================
  Coverage   91.61%   91.61%           
=======================================
  Files          47       47           
  Lines        3433     3436    +3     
=======================================
+ Hits         3145     3148    +3     
  Misses        288      288           
Flag Coverage Δ
unittests 91.61% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
nonebot/rule.py 97.68% <100.00%> (+0.02%) ⬆️

@github-actions github-actions bot temporarily deployed to pull request October 18, 2023 07:50 Inactive
@yanyongyu yanyongyu merged commit 97a57c2 into nonebot:master Oct 18, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

3 participants