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

API for automatically splitting long messages in places where spec allows/requires it #51

Merged
merged 9 commits into from
Nov 13, 2020

Conversation

dwfreed
Copy link
Member

@dwfreed dwfreed commented Nov 6, 2020

This will greatly simplify and deduplicate code in various places, like CAP LS/LIST, NAMES, and WHOIS channel lists.

Fixes #16

@dwfreed dwfreed force-pushed the dwfreed/sendto-one-multiline branch from d19f95e to 2bc579f Compare November 6, 2020 23:51
Copy link
Contributor

@edk0 edk0 left a comment

Choose a reason for hiding this comment

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

The missing (void) is the only serious problem here, and it's easy to address :)

One thing that crossed my mind and doesn't really fit anywhere specific in the diff is that it'd be pretty easy to keep the prefix in the same buffer we use to build items, but I don't see any great benefit beside the warm fuzzy feeling of saving a BUFSIZE.

ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
include/send.h Outdated Show resolved Hide resolved
include/send.h Outdated Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Show resolved Hide resolved
modules/m_monitor.c Outdated Show resolved Hide resolved
@edk0 edk0 linked an issue Nov 8, 2020 that may be closed by this pull request
@dwfreed dwfreed force-pushed the dwfreed/sendto-one-multiline branch from 6a4faa9 to d34ab36 Compare November 8, 2020 19:01
@dwfreed dwfreed marked this pull request as ready for review November 8, 2020 19:12
Copy link
Contributor

@edk0 edk0 left a comment

Choose a reason for hiding this comment

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

I made a start on a test suite for this while reviewing it: 56890e2 —feel free to pull it into the PR if you think it's useful.

ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
ircd/send.c Show resolved Hide resolved
ircd/send.c Show resolved Hide resolved
ircd/send.c Outdated Show resolved Hide resolved
@dwfreed dwfreed force-pushed the dwfreed/sendto-one-multiline branch 2 times, most recently from 7f6f2e9 to 29f72a3 Compare November 9, 2020 02:50
ircd/send.c Show resolved Hide resolved
@dwfreed dwfreed force-pushed the dwfreed/sendto-one-multiline branch from 29f72a3 to 4b958a2 Compare November 9, 2020 23:32
@dwfreed dwfreed requested a review from edk0 November 11, 2020 02:28
Copy link
Contributor

@edk0 edk0 left a comment

Choose a reason for hiding this comment

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

Modulo suggestions I think this is in a pretty good place now.

I think it might be worth raising, since this will be the last good opportunity to filter-branch, the question of whether we could bikeshed the names down a bit. sendto_one_multiline_remote_extra_space doesn't exactly roll off the tongue.

tests/send_multiline1.c Outdated Show resolved Hide resolved
modules/m_cap.c Outdated Show resolved Hide resolved
modules/m_monitor.c Outdated Show resolved Hide resolved
modules/m_monitor.c Outdated Show resolved Hide resolved
modules/m_monitor.c Outdated Show resolved Hide resolved
modules/m_cap.c Outdated Show resolved Hide resolved
@dwfreed dwfreed force-pushed the dwfreed/sendto-one-multiline branch 2 times, most recently from 66e06fb to 20ed37d Compare November 12, 2020 07:52
Copy link
Contributor

@edk0 edk0 left a comment

Choose a reason for hiding this comment

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

🍰

@dwfreed dwfreed merged commit 99b8e2f into main Nov 13, 2020
@dwfreed dwfreed deleted the dwfreed/sendto-one-multiline branch November 13, 2020 00:18
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.

Multiple lines in the output of /findforwards Centralise multiline reply generation
2 participants