From 873e0b90f1869d6ddf889302c981f3f0cb714388 Mon Sep 17 00:00:00 2001 From: John Hernandez <129467592+H3rnand3zzz@users.noreply.github.com> Date: Fri, 15 Dec 2023 09:20:27 +0100 Subject: [PATCH] Fix overscrolling issue Before there was a problem of overscrolling: when messages longer than y axis of the terminal are fetched from the DB, profanity scroll "jumps" to the top, skipping some messages. It's resolved by keeping messages' starting and ending line in the internal profanity buffer, which allows to track proper message positions and to adjust window position accordingly. Message size is now tracked as part of the buffer's record in `_line` variable, which allows calculation of the total buffer size, which is a part of the improved solution for the "underscrolling" problem. Underscrolling problem was fixed in a previous commit d7e46d64fe6bc9926eb404dad6a4803e1fc2e8b2 Short recap of the problem: Despite user scrolling to top/bottom of history, factual position is offset from the intended location Another feature of this commit is a minor change which adds fetching message stanza IDs from the DB. It allows correcting messages fetched from history. Fixes https://github.com/profanity-im/profanity/issues/1934 --- src/database.c | 4 +- src/ui/buffer.c | 98 +++++++++++++++++++++++++++---------------------- src/ui/buffer.h | 7 +++- src/ui/window.c | 60 +++++++++++++++++++----------- 4 files changed, 100 insertions(+), 69 deletions(-) diff --git a/src/database.c b/src/database.c index ce1d1d3d2..6d362bc0a 100644 --- a/src/database.c +++ b/src/database.c @@ -339,7 +339,7 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const char* s auto_gchar gchar* end_date_fmt = end_time ? end_time : g_date_time_format_iso8601(now); auto_sqlite gchar* query = sqlite3_mprintf("SELECT * FROM (" "SELECT COALESCE(B.`message`, A.`message`) AS message, " - "A.`timestamp`, A.`from_jid`, A.`to_jid`, A.`type`, A.`encryption` FROM `ChatLogs` AS A " + "A.`timestamp`, A.`from_jid`, A.`to_jid`, A.`type`, A.`encryption`, A.`stanza_id` FROM `ChatLogs` AS A " "LEFT JOIN `ChatLogs` AS B ON (A.`replaced_by_db_id` = B.`id` AND A.`from_jid` = B.`from_jid`) " "WHERE (A.`replaces_db_id` IS NULL) " "AND ((A.`from_jid` = %Q AND A.`to_jid` = %Q) OR (A.`from_jid` = %Q AND A.`to_jid` = %Q)) " @@ -371,8 +371,10 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const char* s char* to_jid = (char*)sqlite3_column_text(stmt, 3); char* type = (char*)sqlite3_column_text(stmt, 4); char* encryption = (char*)sqlite3_column_text(stmt, 5); + char* id = (char*)sqlite3_column_text(stmt, 6); ProfMessage* msg = message_init(); + msg->id = id ? strdup(id) : NULL; msg->from_jid = jid_create(from); msg->to_jid = jid_create(to_jid); msg->plain = strdup(message ?: ""); diff --git a/src/ui/buffer.c b/src/ui/buffer.c index e14434c2b..d746fd6d1 100644 --- a/src/ui/buffer.c +++ b/src/ui/buffer.c @@ -50,23 +50,30 @@ #include #endif +#include "log.h" #include "ui/window.h" #include "ui/buffer.h" -#define BUFF_SIZE 200 +#define BUFF_SIZE 200 +#define BUFF_SIZE_LINES 800 +#define STRDUP_OR_NULL(str) ((str) ? strdup(str) : NULL) struct prof_buff_t { GSList* entries; + int lines; }; static void _free_entry(ProfBuffEntry* entry); +static ProfBuffEntry* _create_entry(const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos); +static void _buffer_add(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos, gboolean append); ProfBuff buffer_create(void) { ProfBuff new_buff = malloc(sizeof(struct prof_buff_t)); new_buff->entries = NULL; + new_buff->lines = 0; return new_buff; } @@ -84,58 +91,37 @@ buffer_free(ProfBuff buffer) } void -buffer_append(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id) +buffer_append(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos) { - ProfBuffEntry* e = malloc(sizeof(struct prof_buff_entry_t)); - e->show_char = strdup(show_char); - e->pad_indent = pad_indent; - e->flags = flags; - e->theme_item = theme_item; - e->time = g_date_time_ref(time); - e->display_from = display_from ? strdup(display_from) : NULL; - e->from_jid = from_jid ? strdup(from_jid) : NULL; - e->message = strdup(message); - e->receipt = receipt; - if (id) { - e->id = strdup(id); - } else { - e->id = NULL; - } - - if (g_slist_length(buffer->entries) == BUFF_SIZE) { - _free_entry(buffer->entries->data); - buffer->entries = g_slist_delete_link(buffer->entries, buffer->entries); - } - - buffer->entries = g_slist_append(buffer->entries, e); + _buffer_add(buffer, show_char, pad_indent, time, flags, theme_item, display_from, from_jid, message, receipt, id, y_start_pos, y_end_pos, TRUE); } void -buffer_prepend(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id) +buffer_prepend(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos) { - ProfBuffEntry* e = malloc(sizeof(struct prof_buff_entry_t)); - e->show_char = strdup(show_char); - e->pad_indent = pad_indent; - e->flags = flags; - e->theme_item = theme_item; - e->time = g_date_time_ref(time); - e->display_from = display_from ? strdup(display_from) : NULL; - e->from_jid = from_jid ? strdup(from_jid) : NULL; - e->message = strdup(message); - e->receipt = receipt; - if (id) { - e->id = strdup(id); - } else { - e->id = NULL; + _buffer_add(buffer, show_char, pad_indent, time, flags, theme_item, display_from, from_jid, message, receipt, id, y_start_pos, y_end_pos, FALSE); +} + +static void +_buffer_add(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos, gboolean append) +{ + ProfBuffEntry* e = _create_entry(show_char, pad_indent, time, flags, theme_item, display_from, from_jid, message, receipt, id, y_start_pos, y_end_pos); + + buffer->lines += e->_lines; + + while (g_slist_length(buffer->entries) > 11 && (g_slist_length(buffer->entries) >= BUFF_SIZE || buffer->lines > BUFF_SIZE_LINES)) { + GSList* buffer_entry_to_delete = append ? buffer->entries : g_slist_last(buffer->entries); + ProfBuffEntry* entry_to_delete = (ProfBuffEntry*)buffer_entry_to_delete->data; + buffer->lines -= entry_to_delete->_lines; + _free_entry(entry_to_delete); + buffer->entries = g_slist_delete_link(buffer->entries, buffer_entry_to_delete); } - if (g_slist_length(buffer->entries) == BUFF_SIZE) { - GSList* last = g_slist_last(buffer->entries); - _free_entry(last->data); - buffer->entries = g_slist_delete_link(buffer->entries, last); + if (from_jid && y_end_pos == y_start_pos) { + log_warning("Ncurses Overflow! From: %s, pos: %d, ID: %s, message: %s", from_jid, y_end_pos, id, message); } - buffer->entries = g_slist_prepend(buffer->entries, e); + buffer->entries = append ? g_slist_append(buffer->entries, e) : g_slist_prepend(buffer->entries, e); } void @@ -145,6 +131,7 @@ buffer_remove_entry_by_id(ProfBuff buffer, const char* const id) while (entries) { ProfBuffEntry* entry = entries->data; if (entry->id && (g_strcmp0(entry->id, id) == 0)) { + buffer->lines -= entry->_lines; _free_entry(entry); buffer->entries = g_slist_delete_link(buffer->entries, entries); break; @@ -157,6 +144,8 @@ void buffer_remove_entry(ProfBuff buffer, int entry) { GSList* node = g_slist_nth(buffer->entries, entry); + ProfBuffEntry* e = node->data; + buffer->lines -= e->_lines; _free_entry(node->data); buffer->entries = g_slist_delete_link(buffer->entries, node); } @@ -201,6 +190,27 @@ buffer_get_entry_by_id(ProfBuff buffer, const char* const id) return NULL; } +static ProfBuffEntry* +_create_entry(const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const from_jid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos) +{ + ProfBuffEntry* e = malloc(sizeof(struct prof_buff_entry_t)); + e->show_char = STRDUP_OR_NULL(show_char); + e->pad_indent = pad_indent; + e->flags = flags; + e->theme_item = theme_item; + e->time = g_date_time_ref(time); + e->display_from = STRDUP_OR_NULL(display_from); + e->from_jid = STRDUP_OR_NULL(from_jid); + e->message = STRDUP_OR_NULL(message); + e->receipt = receipt; + e->id = STRDUP_OR_NULL(id); + e->y_start_pos = y_start_pos; + e->y_end_pos = y_end_pos; + e->_lines = e->y_end_pos - e->y_start_pos; + + return e; +} + static void _free_entry(ProfBuffEntry* entry) { diff --git a/src/ui/buffer.h b/src/ui/buffer.h index 591c48812..a5f863d25 100644 --- a/src/ui/buffer.h +++ b/src/ui/buffer.h @@ -52,6 +52,9 @@ typedef struct prof_buff_entry_t // pointer because it could be a unicode symbol as well gchar* show_char; int pad_indent; + int y_start_pos; + int y_end_pos; + int _lines; GDateTime* time; int flags; theme_item_t theme_item; @@ -69,8 +72,8 @@ typedef struct prof_buff_t* ProfBuff; ProfBuff buffer_create(); void buffer_free(ProfBuff buffer); -void buffer_append(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const barejid, const char* const message, DeliveryReceipt* receipt, const char* const id); -void buffer_prepend(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const barejid, const char* const message, DeliveryReceipt* receipt, const char* const id); +void buffer_append(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const barejid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos); +void buffer_prepend(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* time, int flags, theme_item_t theme_item, const char* const display_from, const char* const barejid, const char* const message, DeliveryReceipt* receipt, const char* const id, int y_start_pos, int y_end_pos); void buffer_remove_entry_by_id(ProfBuff buffer, const char* const id); void buffer_remove_entry(ProfBuff buffer, int entry); int buffer_size(ProfBuff buffer); diff --git a/src/ui/window.c b/src/ui/window.c index b0452091e..6bc3f92c3 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -630,16 +630,14 @@ win_free(ProfWin* window) void win_page_up(ProfWin* window, int scroll_size) { - int rows = getmaxy(stdscr); - int total_rows = getcury(window->layout->win); - int page_space = rows - 4; int* page_start = &(window->layout->y_pos); int page_start_initial = *page_start; + int total_rows = getcury(window->layout->win); + int page_space = getmaxy(stdscr) - 4; if (scroll_size == 0) scroll_size = page_space; win_scroll_state_t* scroll_state = &window->scroll_state; *scroll_state = (*scroll_state == WIN_SCROLL_REACHED_BOTTOM) ? WIN_SCROLL_INNER : *scroll_state; - *page_start -= scroll_size; if (*page_start == -scroll_size && window->type == WIN_CHAT) { @@ -656,12 +654,18 @@ win_page_up(ProfWin* window, int scroll_size) win_print_loading_history(window); iq_mam_request_older(chatwin); } + + int buff_size = buffer_size(window->layout->buffer); + int offset_entry_id = buff_size > 10 ? 10 : buff_size - 1; + int offset = buffer_get_entry(window->layout->buffer, offset_entry_id)->y_end_pos; + *page_start = offset - page_space; } } // went past beginning, show first page - if (*page_start < 0) + if (*page_start < 0) { *page_start = 0; + } window->layout->paged = 1; // update only if position has changed @@ -678,10 +682,9 @@ win_page_up(ProfWin* window, int scroll_size) void win_page_down(ProfWin* window, int scroll_size) { - int rows = getmaxy(stdscr); - int* page_start = &(window->layout->y_pos); int total_rows = getcury(window->layout->win); - int page_space = rows - 4; + int* page_start = &(window->layout->y_pos); + int page_space = getmaxy(stdscr) - 4; int page_start_initial = *page_start; if (scroll_size == 0) scroll_size = page_space; @@ -694,18 +697,23 @@ win_page_down(ProfWin* window, int scroll_size) if ((*page_start == total_rows || (*page_start == page_space && *page_start >= total_rows)) && window->type == WIN_CHAT) { int bf_size = buffer_size(window->layout->buffer); if (bf_size > 0) { - auto_gchar gchar* start = g_date_time_format_iso8601(buffer_get_entry(window->layout->buffer, bf_size - 1)->time); + ProfBuffEntry* last_entry = buffer_get_entry(window->layout->buffer, bf_size - 1); + auto_gchar gchar* start = g_date_time_format_iso8601(last_entry->time); GDateTime* now = g_date_time_new_now_local(); - gchar* end = g_date_time_format_iso8601(now); - // end is free'd inside - if (*scroll_state != WIN_SCROLL_REACHED_BOTTOM && !chatwin_db_history((ProfChatWin*)window, start, end, FALSE)) { + gchar* end_date = g_date_time_format_iso8601(now); + if (*scroll_state != WIN_SCROLL_REACHED_BOTTOM && !chatwin_db_history((ProfChatWin*)window, start, end_date, FALSE)) { *scroll_state = WIN_SCROLL_REACHED_BOTTOM; } g_date_time_unref(now); + + int offset = last_entry->y_end_pos - 1; + *page_start = offset; } } + total_rows = getcury(window->layout->win); + // only got half a screen, show full screen if ((total_rows - (*page_start)) < page_space) *page_start = total_rows - page_space; @@ -715,7 +723,7 @@ win_page_down(ProfWin* window, int scroll_size) *page_start = total_rows - page_space - 1; window->layout->paged = 1; - // update only if position has changed + if (page_start_initial != *page_start) { win_update_virtual(window); } @@ -1487,10 +1495,11 @@ win_print_history(ProfWin* window, const ProfMessage* const message) auto_gchar gchar* display_name = get_display_name(message, &flags); auto_char char* ch = get_show_char(message->enc); - buffer_append(window->layout->buffer, ch, 0, message->timestamp, flags, THEME_TEXT_HISTORY, display_name, NULL, message->plain, NULL, NULL); wins_add_urls_ac(window, message, FALSE); wins_add_quotes_ac(window, message->plain, FALSE); + int y_start_pos = getcury(window->layout->win); _win_print_internal(window, ch, 0, message->timestamp, flags, THEME_TEXT_HISTORY, display_name, message->plain, NULL); + buffer_append(window->layout->buffer, ch, 0, message->timestamp, flags, THEME_TEXT_HISTORY, display_name, message->from_jid->barejid, message->plain, NULL, message->id, y_start_pos, getcury(window->layout->win)); inp_nonblocking(TRUE); g_date_time_unref(message->timestamp); @@ -1506,10 +1515,11 @@ win_print_old_history(ProfWin* window, const ProfMessage* const message) auto_char char* ch = get_show_char(message->enc); - buffer_prepend(window->layout->buffer, ch, 0, message->timestamp, flags, THEME_TEXT_HISTORY, display_name, NULL, message->plain, NULL, NULL); + int y_start_pos = getcury(window->layout->win); wins_add_urls_ac(window, message, TRUE); wins_add_quotes_ac(window, message->plain, TRUE); _win_print_internal(window, ch, 0, message->timestamp, flags, THEME_TEXT_HISTORY, display_name, message->plain, NULL); + buffer_prepend(window->layout->buffer, ch, 0, message->timestamp, flags, THEME_TEXT_HISTORY, display_name, message->from_jid->barejid, message->plain, NULL, message->id, y_start_pos, getcury(window->layout->win)); inp_nonblocking(TRUE); g_date_time_unref(message->timestamp); @@ -1522,8 +1532,9 @@ win_println_va_internal(ProfWin* window, theme_item_t theme_item, int pad, int f auto_gchar gchar* msg = g_strdup_vprintf(message, arg); - buffer_append(window->layout->buffer, show_char, pad, timestamp, flags, theme_item, "", NULL, msg, NULL, NULL); + int y_start_pos = getcury(window->layout->win); _win_print_internal(window, show_char, pad, timestamp, flags, theme_item, "", msg, NULL); + buffer_append(window->layout->buffer, show_char, pad, timestamp, flags, theme_item, "", NULL, msg, NULL, NULL, y_start_pos, getcury(window->layout->win)); inp_nonblocking(TRUE); g_date_time_unref(timestamp); @@ -1616,8 +1627,9 @@ win_print_outgoing_with_receipt(ProfWin* window, const char* show_char, const ch if (_win_correct(window, message, id, replace_id, myjid)) { free(receipt); // TODO: probably we should use this in _win_correct() } else { - buffer_append(window->layout->buffer, show_char, 0, time, 0, THEME_TEXT_ME, from, myjid, message, receipt, id); + int y_start_pos = getcury(window->layout->win); _win_print_internal(window, show_char, 0, time, 0, THEME_TEXT_ME, from, message, receipt); + buffer_append(window->layout->buffer, show_char, 0, time, 0, THEME_TEXT_ME, from, myjid, message, receipt, id, y_start_pos, getcury(window->layout->win)); } // TODO: cross-reference.. this should be replaced by a real event-based system @@ -1676,8 +1688,9 @@ _win_printf(ProfWin* window, const char* show_char, int pad_indent, GDateTime* t auto_gchar gchar* msg = g_strdup_vprintf(message, arg); - buffer_append(window->layout->buffer, show_char, pad_indent, timestamp, flags, theme_item, display_from, from_jid, msg, NULL, message_id); + int y_start_pos = getcury(window->layout->win); _win_print_internal(window, show_char, pad_indent, timestamp, flags, theme_item, display_from, msg, NULL); + buffer_append(window->layout->buffer, show_char, pad_indent, timestamp, flags, theme_item, display_from, from_jid, msg, NULL, message_id, y_start_pos, getcury(window->layout->win)); inp_nonblocking(TRUE); g_date_time_unref(timestamp); @@ -1956,13 +1969,13 @@ win_print_trackbar(ProfWin* window) void win_redraw(ProfWin* window) { - int size; + int size = buffer_size(window->layout->buffer); werase(window->layout->win); - size = buffer_size(window->layout->buffer); for (int i = 0; i < size; i++) { ProfBuffEntry* e = buffer_get_entry(window->layout->buffer, i); + e->y_start_pos = getcury(window->layout->win); if (e->display_from == NULL && e->message && e->message[0] == '-') { // just an indicator to print the trackbar/separator not the actual message win_print_trackbar(window); @@ -1970,6 +1983,7 @@ win_redraw(ProfWin* window) // regular thing to print _win_print_internal(window, e->show_char, e->pad_indent, e->time, e->flags, e->theme_item, e->display_from, e->message, e->receipt); } + e->y_end_pos = getcury(window->layout->win); } } @@ -1985,7 +1999,8 @@ win_print_loading_history(ProfWin* window) timestamp = g_date_time_new_now_local(); } - buffer_prepend(window->layout->buffer, "-", 0, timestamp, NO_DATE, THEME_ROOMINFO, NULL, NULL, LOADING_MESSAGE, NULL, NULL); + int cur_y = getcury(window->layout->win); + buffer_prepend(window->layout->buffer, "-", 0, timestamp, NO_DATE, THEME_ROOMINFO, NULL, NULL, LOADING_MESSAGE, NULL, NULL, cur_y, cur_y + 1); if (is_buffer_empty) g_date_time_unref(timestamp); @@ -2203,7 +2218,8 @@ win_insert_last_read_position_marker(ProfWin* window, char* id) // the trackbar/separator will actually be print in win_redraw(). // this only puts it in the buffer and win_redraw() will interpret it. // so that we have the correct length even when resizing. - buffer_append(window->layout->buffer, " ", 0, time, 0, THEME_TEXT, NULL, NULL, "-", NULL, id); + int y_start_pos = getcury(window->layout->win); + buffer_append(window->layout->buffer, " ", 0, time, 0, THEME_TEXT, NULL, NULL, "-", NULL, id, y_start_pos, getcury(window->layout->win)); win_redraw(window); g_date_time_unref(time);