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

Hyperlink on news #30

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

LightDestory
Copy link
Member

@LightDestory LightDestory commented Apr 21, 2022

This PR should address #28

After this change, the footer will be always appended to the news with a hypertext linked to the url.

I performed the most minimal changes due to unfamiliar code base.

@LightDestory LightDestory requested review from TendTo, Helias and aegroto and removed request for TendTo and Helias April 21, 2022 11:48
@LightDestory
Copy link
Member Author

The actual change is f4263c4


# If message content is too long, cut it and add a footer
if len(content) > max_len:
split_index = max_len - 1

while content[split_index] != ' ':
split_index = split_index - 1

content = "{}{}".format(content[:split_index], config_map["max_length_footer"])
footer: str = str(config_map["max_length_footer"]).replace("%PLACE_HOLDER%", url)
Copy link
Member

Choose a reason for hiding this comment

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

The type can be easily inferred by the cast and following 'replace' function

@LightDestory
Copy link
Member Author

keep in mind that you removed the content line from the if, so now it is outside from the if, are you sure about it?

Yes, as stated on the opening message the footer is now always appended

@Helias
Copy link
Member

Helias commented May 1, 2022

is this ready?

@LightDestory
Copy link
Member Author

It should, I don't know how to test such change.

@Helias
Copy link
Member

Helias commented Jun 17, 2022

why not?

@Helias
Copy link
Member

Helias commented Nov 12, 2022

merge conflicts

@Helias
Copy link
Member

Helias commented Apr 2, 2023

:(

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.

3 participants