Skip to content

replace kwargs double backslash for multiline messages#294

Open
leonbuchnerbd wants to merge 6 commits intojadolg:masterfrom
leonbuchnerbd:bug/multiline_messages
Open

replace kwargs double backslash for multiline messages#294
leonbuchnerbd wants to merge 6 commits intojadolg:masterfrom
leonbuchnerbd:bug/multiline_messages

Conversation

@leonbuchnerbd
Copy link

rocketchat_API/APISections/base.py line 86

if you want to use the api to send multiline messages, you have to adapt the kwargs arguments.

the kwargs arguments are json serialized and have problems with escape sequences.

I fixed this here.

@sonarqubecloud
Copy link

Copy link
Owner

@jadolg jadolg left a comment

Choose a reason for hiding this comment

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

Hi @leonbuchnerbd
thank you for your contribution.
I left you a comment here with some changes I think are worth doing and I think the linter isn't happy. That should be easy to fix with black.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 4, 2025

@leonbuchnerbd leonbuchnerbd requested a review from jadolg September 4, 2025 11:20
@leonbuchnerbd
Copy link
Author

i updated your change request. i hope that will works now. ty

Copy link
Owner

@jadolg jadolg left a comment

Choose a reason for hiding this comment

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

Tests are broken and the linter is also not passing. Would you mind taking a look?

@jadolg
Copy link
Owner

jadolg commented Feb 15, 2026

I was having another look now and it overall looks good to me. I'll save you the effort of merging the bunch of things I've added over the past months and will just port your changes on a new branch.

@jadolg
Copy link
Owner

jadolg commented Feb 15, 2026

It seems I've misunderstood the issue.
I first added the tests to see what actually happens and I realized I don't understand why is the input double-escaped.
If the text is double-escaped before entering the method, the responsibility to clean it up should not be in it as it would be doing more than one thing at the same time.
I also tried with normally-escaped and multiline strings and the results in the interface seems just fine (except for the attachments in which 2 new lines are needed instead of one but double escaping does not fix this)

PS: the problem with attachments was a markdown issue

image image

@leonbuchnerbd
Copy link
Author

so should i fix this tests or should we close this?

@jadolg
Copy link
Owner

jadolg commented Feb 20, 2026

so should i fix this tests or should we close this?

I want to understand first if the problem you found is still there cause I either don't understand it or can't reproduce it. Mind explaining how to reproduce it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants