numberoverzero / bottom

asyncio-based rfc2812-compliant IRC Client
http://bottom-docs.readthedocs.io
MIT License
74 stars 23 forks source link

Handling MODE returned by server #52

Closed elazkani closed 6 years ago

elazkani commented 6 years ago

Description

Transforming the MODE returned by the server into USERMODE or CHANNELMODE triggers depending on the type of parameters they are.

NOTE: This has been tested on ircu2 and derivatives

$ coverage run --branch --source=bottom -m py.test --ignore=tests/integ
======================================================== test session starts ========================================================
platform darwin -- Python 3.6.1, pytest-3.2.2, py-1.4.34, pluggy-0.4.0
rootdir: /Users/Elijah/sandbox/bottom, inifile:
collected 107 items

tests/unit/test_client.py ....................
tests/unit/test_pack.py .................................................
tests/unit/test_protocol.py .........
tests/unit/test_unpack.py .............................

==================================================== 107 passed in 0.52 seconds =====================================================

Test code

    @bot.on("USERMODE")
    def umode(**kwargs):
        logger.debug("USERMODE {}".format(kwargs))

    @bot.on("CHANNELMODE")
    def cmode(**kwargs):
        logger.debug("CHANNELMODE {}".format(kwargs))

Sample output:

USERMODE {'nick': 'nck', 'user': 'usr', 'host': 'hst', 'modes': '+iwx'}
CHANNELMODE {'nick': 'nck', 'user': 'usr', 'host': 'hst', 'channel': '#ch', 'modes': '+o', 'params': 'bot'}
numberoverzero commented 6 years ago

Thanks for the PR! On first review it looks good but I remember some grumbling about MODE before. Maybe it's easier than I thought.

Historically it's been about a week until I get through a PR; that's not for lack of interest, just a limitation of free time.

elazkani commented 6 years ago

@numberoverzero that's not a problem. I tried to keep it consistent with the pack function. I just noticed that Travis is failing due to flake8, maybe I did not test it enough. I'll try to fix that issue and apply another commit.

elazkani commented 6 years ago

I found 2 edge cases that I added. Now the output will look something like the following

CHANNELMODE {'nick': 'nck', 'user': 'usr', 'host': 'hst', 'channel': '#ch', 'modes': '+m', 'params': ''}
CHANNELMODE {'nick': 'nck', 'user': 'usr', 'host': 'hst', 'channel': '#ch', 'modes': '+o', 'params': ['params']}

And I only returned it in this format to keep compatibility with previous code.

codecov[bot] commented 6 years ago

Codecov Report

Merging #52 into master will increase coverage by 0.05%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
+ Coverage   98.87%   98.92%   +0.05%     
==========================================
  Files           5        5              
  Lines         443      464      +21     
  Branches      123      128       +5     
==========================================
+ Hits          438      459      +21     
  Misses          3        3              
  Partials        2        2
Impacted Files Coverage Δ
bottom/__init__.py 100% <100%> (ø) :arrow_up:
bottom/unpack.py 100% <100%> (ø) :arrow_up:

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 66d01cd...252c8d2. Read the comment docs.

elazkani commented 6 years ago

So I bumped the version as well so that people using this will not break their stuff. I wasn't sure if you wanted to bump the minor or the major so I stuck with the minor. Also, forgive the typo in the commit message, I don't want to fix it by rewriting history that would cause travis to go crazy, it can be fixed by squashing before merging I guess. =)

numberoverzero commented 6 years ago

Thanks for your contribution! 2.1.2 is live