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.

Sign up for GitHub

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

Jump to bottom

Fix prof_pre_chat_message_display does not replace sent messages + minor refactoring #1918

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Fix prof_pre_chat_message_display does not replace sent messages + minor refactoring #1918

merged 2 commits into from
Nov 20, 2023

Conversation

H3rnand3zzz
Copy link
Contributor

@H3rnand3zzz H3rnand3zzz commented Nov 9, 2023 •
edited
Loading

How to test the functionality

Fix #1917

@jubalh jubalh self-requested a review November 10, 2023 21:13
@jubalh jubalh added this to the next milestone Nov 10, 2023
@H3rnand3zzz
Copy link
Contributor Author

H3rnand3zzz commented Nov 11, 2023 •
edited
Loading

Revision-1

Rebased on master

Note for myself: never use github's rebase anymore

@H3rnand3zzz
Copy link
Contributor Author

H3rnand3zzz commented Nov 11, 2023 •
edited
Loading

What a mess... Further revision is intended just to clean it up.

@H3rnand3zzz H3rnand3zzz force-pushed the fix/plugins-pre-display branch 3 times, most recently from 010efbd to 4d219ce Compare November 11, 2023 21:12
@H3rnand3zzz
Copy link
Contributor Author

H3rnand3zzz commented Nov 11, 2023 •
edited
Loading

Revision-2

Change memory handling to suggested by @sjaeckel method.

Revision-3

  • Set commit message back to more descriptive version
  • Remove unwanted minor change (autoformatted)

@H3rnand3zzz
Refactor plugins_pre_chat_message_display for Memory Management
16ed7cc 
Refactor function to enhance memory handling, addressing
temporary workaround introduced in 2e0adbd. Adjustments
ensure cleaner code and maintainability.

Part of the improvements suggested by @sjaeckel.
@H3rnand3zzz
Fix: add plugins_pre_chat_message_display call for outgoing messages
b19f668 
Ensure consistent invocation of `plugins_pre_chat_message_display`
for outgoing messages. Before the change, the function was not
called for sent messages upon sending, but only on fetching
sent messages from DB.

Fix profanity-im#1917
@H3rnand3zzz
Copy link
Contributor Author

Revision 4

  • Further improvements based on @sjaeckel suggestions.
  • Significantly improved commit messages.

jubalh
jubalh approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@jubalh jubalh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me. Since jaeckel was involved I will also set him as reviewer and wait for it.

@jubalh jubalh requested a review from sjaeckel November 13, 2023 08:26
@jubalh jubalh merged commit 8ba248c into profanity-im:master Nov 20, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jubalh jubalh jubalh approved these changes

@sjaeckel sjaeckel sjaeckel approved these changes

Assignees

@H3rnand3zzz H3rnand3zzz

Labels
improvement
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.

prof_pre_chat_message_display does not replace sent messages
3 participants
@H3rnand3zzz @sjaeckel @jubalh