Download raw body.
tog: limit feature
On 22-08-27 05:19PM, Mikhail wrote: > On Sat, Aug 20, 2022 at 09:26:50PM +1000, Mark Jamsek wrote: > > On 22-08-19 07:51PM, Mikhail wrote: > > > On Sat, Aug 20, 2022 at 02:27:11AM +1000, Mark Jamsek wrote: > > > > On 22-08-19 07:09PM, Mikhail wrote: > > > > > On Sat, Aug 20, 2022 at 01:55:22AM +1000, Mark Jamsek wrote: > > > > > > There're a few minor style issues that crept in with the indentation > > > > > > changes that I've annotated inline, but otherwise this is looking good. > > > > > > Thanks, Mikhail! This will be a useful addition in my togging. > > > > > > > > > > Fixed. > > > > > > > > You missed the 80 column wrap on tog.c:2228 > > > > > > > > 2228 TAILQ_INSERT_TAIL(&a->limit_commits->head, matched, > > > > 2229 entry); > > > > > > > > should be: > > > > > > > > 2228 TAILQ_INSERT_TAIL(&a->limit_commits->head, > > > > 2229 matched, entry); > > > > > > > > And op caught the missed malloc() checks in queue_commits() and > > > > limit_log_view() annotated below. > > > > > > Thanks, done. > > > > This looks good to me! ok > > Hello, Omar found a bug in previous version: > > & foo <enter> Ctrl+L & <enter> q - segfault. > > Reason for that is the use of memcpy() for copying commits from > real_commits to limit_commits. This version of the patch adds > got_copy_commit function, which properly copies commit fields. > > Another change - we don't store saved_*_entries variables, because if > the user hits ^L old queue get free()'ed and saved vars will point to a > garbage, instead of that we always select first entry of the queue while > switching the views. I also think it is arguably more intuitive to reset the selected entry to the first in the queue when switching to/from limit views, so this is fine by me. The updated diff indeed fixes the bug op reported, and works great! A couple minor nits annotated inline but otherwise this looks good. > diff refs/heads/main refs/heads/limit5 > commit - f0680473a7db1e5941bffdc2ab5f80ddec209122 > commit + b37e822a9a348aefb018d62fe24e2ff862cc6be3 > blob - 1cd6f349912d3e03ebbdccfd4beeeb54663af7fb > blob + 24a4ff4a4466649e0fca9de9b1cb9fbf0bddb6bc > --- include/got_object.h > +++ include/got_object.h > @@ -352,3 +352,5 @@ const struct got_error *got_object_commit_add_parent(s > const struct got_error *got_object_tag_create(struct got_object_id **, > const char *, struct got_object_id *, const char *, > time_t, const char *, const char *, struct got_repository *, int verbosity); > + > +struct got_commit_object *got_copy_commit(struct got_commit_object *); > blob - 12ae8e569a5cb16540cc4d70ad34d56c0964258a > blob + 0507b2a89edaec5f470a1f15d2c4006c4d370c74 > --- lib/object.c > +++ lib/object.c > @@ -2352,3 +2352,57 @@ done: > free(path_packfile); > return err; > } > + > +struct got_commit_object * > +got_copy_commit(struct got_commit_object *commit) > +{ > + const struct got_object_id_queue *parent_ids; > + struct got_commit_object *newcommit; > + struct got_object_id *newtree_id; > + struct got_object_qid *qidp; > + > + newcommit = calloc(1, sizeof(*newcommit)); > + if (newcommit == NULL) > + return NULL; > + > + newtree_id = calloc(1, sizeof(*newtree_id)); > + if (newtree_id == NULL) { > + free(newcommit); > + return NULL; > + } > + newcommit->tree_id = newtree_id; > + memcpy(newcommit->tree_id->sha1, commit->tree_id->sha1, > + SHA1_DIGEST_LENGTH); > + parent_ids = got_object_commit_get_parent_ids(commit); > + STAILQ_INIT(&newcommit->parent_ids); > + newcommit->nparents = 0; > + STAILQ_FOREACH(qidp, parent_ids, entry) { > + struct got_object_qid *qid; > + > + qid = malloc(sizeof(*qid)); > + if (qid == NULL) { > + free(newcommit); > + free(newtree_id); > + return NULL; > + } > + > + qid->data = NULL; > + > + memcpy(qid->id.sha1, qidp->id.sha1, SHA1_DIGEST_LENGTH); > + STAILQ_INSERT_TAIL(&newcommit->parent_ids, qid, entry); > + newcommit->nparents++; > + } > + newcommit->author = strdup(commit->author); > + newcommit->author_time = commit->author_time; > + newcommit->author_gmtoff = commit->author_gmtoff; > + newcommit->committer = strdup(commit->committer); > + newcommit->committer_time = commit->committer_time; > + newcommit->committer_gmtoff = commit->committer_gmtoff; > + newcommit->logmsg = strdup(commit->logmsg); Please check the result of strdup(). > + newcommit->refcnt = 1; > + newcommit->flags = commit->flags; > + > + return newcommit; > +} > blob - cbd5233dd61d780f245c5bcb24d95b030a5a7676 > blob + 5ca270df408b1c24da4ce5567e686412705ded4f > --- tog/tog.1 > +++ tog/tog.1 > @@ -220,6 +220,12 @@ This can then be used to open a new > view for arbitrary branches and tags. > .It Cm @ > Toggle between showing the author and the committer name. > +.It Cm & > +Prompt for a pattern to display a log view limited to the subset of > +commits matching the provided pattern, which is an extended regular > +expression matched against a commit's author name, committer name, > +log message, and commit ID SHA1 hash. > +Use empty string as a pattern to display all commits. > .El > .Pp > The options for > blob - 6ec8b5f0c155fafe55ee2aab0653d259876e6254 > blob + a534f22f8961030669ef846ce5ea4d2f4c78b7a5 > --- tog/tog.c > +++ tog/tog.c > @@ -344,7 +344,7 @@ struct tog_log_thread_args { > int commits_needed; > int load_all; > struct got_commit_graph *graph; > - struct commit_queue *commits; > + struct commit_queue *real_commits; > const char *in_repo_path; > struct got_object_id *start_id; > struct got_repository *repo; > @@ -356,13 +356,18 @@ struct tog_log_thread_args { > int *searching; > int *search_next_done; > regex_t *regex; > + int *limiting; > + int limit_match; > + regex_t *limit_regex; > + struct commit_queue *limit_commits; > }; > > struct tog_log_view_state { > - struct commit_queue commits; > + struct commit_queue *commits; > struct commit_queue_entry *first_displayed_entry; > struct commit_queue_entry *last_displayed_entry; > struct commit_queue_entry *selected_entry; > + struct commit_queue real_commits; > int selected; > char *in_repo_path; > char *head_ref_name; > @@ -376,6 +381,9 @@ struct tog_log_view_state { > struct commit_queue_entry *search_entry; > struct tog_colors colors; > int use_committer; > + int limit_view; > + regex_t limit_regex; > + struct commit_queue limit_commits; > }; > > #define TOG_COLOR_DIFF_MINUS 1 > @@ -935,8 +943,8 @@ resize_log_view(struct tog_view *view, int increase) > * Request commits to account for the increased > * height so we have enough to populate the view. > */ > - if (s->commits.ncommits < n) { > - view->nscrolled = n - s->commits.ncommits + increase + 1; > + if (s->commits->ncommits < n) { > + view->nscrolled = n - s->commits->ncommits + increase + 1; > err = request_log_commits(view); > } > > @@ -2167,6 +2175,7 @@ queue_commits(struct tog_log_thread_args *a) > struct got_object_id *id; > struct got_commit_object *commit; > struct commit_queue_entry *entry; > + int limit_match = 0; > int errcode; > > err = got_commit_graph_iter_next(&id, a->graph, a->repo, > @@ -2190,17 +2199,62 @@ queue_commits(struct tog_log_thread_args *a) > break; > } > > - entry->idx = a->commits->ncommits; > - TAILQ_INSERT_TAIL(&a->commits->head, entry, entry); > - a->commits->ncommits++; > + entry->idx = a->real_commits->ncommits; > + TAILQ_INSERT_TAIL(&a->real_commits->head, entry, entry); > + a->real_commits->ncommits++; > > + if (*a->limiting) { > + err = match_commit(&limit_match, id, commit, > + a->limit_regex); > + if (err) > + break; > + > + if (limit_match) { > + struct commit_queue_entry *matched; > + struct got_object_id *newid; > + > + matched = > + malloc(sizeof(struct commit_queue_entry)); > + if (matched == NULL) > + return got_error_from_errno("malloc"); > + > + newid = calloc(1, sizeof(*newid)); > + if (newid == NULL) { > + free(matched); > + return got_error_from_errno("calloc"); > + } > + matched->id = newid; > + memcpy(matched->id->sha1, entry->id->sha1, > + SHA1_DIGEST_LENGTH); > + matched->commit = > + got_copy_commit(entry->commit); And please check the result of got_copy_commit(). > + matched->idx = a->limit_commits->ncommits; > + TAILQ_INSERT_TAIL(&a->limit_commits->head, > + matched, entry); > + a->limit_commits->ncommits++; > + } > + > + /* > + * This is how we signal log_thread() that we have found > + * a match, and that it should be counted as a new entry > + * for the view. > + */ > + a->limit_match = limit_match; > + } > + > if (*a->searching == TOG_SEARCH_FORWARD && > !*a->search_next_done) { > int have_match; > err = match_commit(&have_match, id, commit, a->regex); > if (err) > break; > - if (have_match) > + > + if (*a->limiting) { > + if (limit_match && have_match) > + *a->search_next_done = > + TOG_SEARCH_HAVE_MORE; > + } else if (have_match) > *a->search_next_done = TOG_SEARCH_HAVE_MORE; > } > > @@ -2271,7 +2325,7 @@ draw_commits(struct tog_view *view) > > if (s->thread_args.commits_needed > 0 || s->thread_args.load_all) { > if (asprintf(&ncommits_str, " [%d/%d] %s", > - entry ? entry->idx + 1 : 0, s->commits.ncommits, > + entry ? entry->idx + 1 : 0, s->commits->ncommits, > (view->searching && !view->search_next_done) ? > "searching..." : "loading...") == -1) { > err = got_error_from_errno("asprintf"); > @@ -2279,6 +2333,7 @@ draw_commits(struct tog_view *view) > } > } else { > const char *search_str = NULL; > + const char *limit_str = NULL; > > if (view->searching) { > if (view->search_next_done == TOG_SEARCH_NO_MORE) > @@ -2289,10 +2344,14 @@ draw_commits(struct tog_view *view) > search_str = "searching..."; > } > > - if (asprintf(&ncommits_str, " [%d/%d] %s", > - entry ? entry->idx + 1 : 0, s->commits.ncommits, > + if (s->limit_view && s->commits->ncommits == 0) > + limit_str = "no limit matches found"; > + > + if (asprintf(&ncommits_str, " [%d/%d] %s %s", > + entry ? entry->idx + 1 : 0, s->commits->ncommits, > search_str ? search_str : > - (refs_str ? refs_str : "")) == -1) { > + (refs_str ? refs_str : ""), > + limit_str ? limit_str : "") == -1) { > err = got_error_from_errno("asprintf"); > goto done; > } > @@ -2419,7 +2478,7 @@ log_scroll_up(struct tog_log_view_state *s, int maxscr > struct commit_queue_entry *entry; > int nscrolled = 0; > > - entry = TAILQ_FIRST(&s->commits.head); > + entry = TAILQ_FIRST(&s->commits->head); > if (s->first_displayed_entry == entry) > return; > > @@ -2504,13 +2563,13 @@ log_scroll_down(struct tog_view *view, int maxscroll) > return NULL; > > ncommits_needed = s->last_displayed_entry->idx + 1 + maxscroll; > - if (s->commits.ncommits < ncommits_needed && > + if (s->commits->ncommits < ncommits_needed && > !s->thread_args.log_complete) { > /* > * Ask the log thread for required amount of commits. > */ > s->thread_args.commits_needed += > - ncommits_needed - s->commits.ncommits; > + ncommits_needed - s->commits->ncommits; > err = trigger_log_thread(view, 1); > if (err) > return err; > @@ -2750,8 +2809,13 @@ log_thread(void *arg) > goto done; > err = NULL; > done = 1; > - } else if (a->commits_needed > 0 && !a->load_all) > - a->commits_needed--; > + } else if (a->commits_needed > 0 && !a->load_all) { > + if (*a->limiting) { > + if (a->limit_match) > + a->commits_needed--; > + } else > + a->commits_needed--; > + } > > errcode = pthread_mutex_lock(&tog_mutex); > if (errcode) { > @@ -2760,10 +2824,14 @@ log_thread(void *arg) > goto done; > } else if (*a->quit) > done = 1; > - else if (*a->first_displayed_entry == NULL) { > + else if (*a->limiting && *a->first_displayed_entry == NULL) { > *a->first_displayed_entry = > - TAILQ_FIRST(&a->commits->head); > + TAILQ_FIRST(&a->limit_commits->head); > *a->selected_entry = *a->first_displayed_entry; > + } else if (*a->first_displayed_entry == NULL) { > + *a->first_displayed_entry = > + TAILQ_FIRST(&a->real_commits->head); > + *a->selected_entry = *a->first_displayed_entry; > } > > errcode = pthread_cond_signal(&a->commit_loaded); > @@ -2867,7 +2935,8 @@ close_log_view(struct tog_view *view) > if (errcode && err == NULL) > err = got_error_set_errno(errcode, "pthread_cond_destroy"); > > - free_commits(&s->commits); > + free_commits(&s->limit_commits); > + free_commits(&s->real_commits); > free(s->in_repo_path); > s->in_repo_path = NULL; > free(s->start_id); > @@ -2877,7 +2946,133 @@ close_log_view(struct tog_view *view) > return err; > } > > +/* > + * We use two queues to implement the limit feature: first consists of commits > + * matching the current limit_regex; second is the real queue of all known > + * commits (real_commits). When the user starts limiting, we swap queues such > + * that all movement and displaying functionality works with very slight change. > + */ > static const struct got_error * > +limit_log_view(struct tog_view *view) > +{ > + struct tog_log_view_state *s = &view->state.log; > + struct commit_queue_entry *entry; > + struct tog_view *v = view; > + const struct got_error *err = NULL; > + char pattern[1024]; > + int ret; > + > + if (view_is_hsplit_top(view)) > + v = view->child; > + else if (view->mode == TOG_VIEW_SPLIT_VERT && view->parent) > + v = view->parent; > + > + /* Get the pattern */ > + wmove(v->window, v->nlines - 1, 0); > + wclrtoeol(v->window); > + mvwaddstr(v->window, v->nlines - 1, 0, "limit: "); > + nodelay(v->window, FALSE); > + nocbreak(); > + echo(); > + ret = wgetnstr(v->window, pattern, sizeof(pattern)); > + cbreak(); > + noecho(); > + nodelay(v->window, TRUE); > + if (ret == ERR) > + return NULL; > + > + if (*pattern == '\0') { > + /* > + * Safety measure for the situation where the user resets limit > + * without previously limiting anything. > + */ > + if (!s->limit_view) > + return NULL; > + > + /* > + * User could have pressed Ctrl+L, which refreshed the commit > + * queues, it means we can't save previously (before limit took > + * place) displayed entries, because they would point to already > + * free'ed memory, so we are forced to always select first entry > + * of the queue. > + */ > + s->commits = &s->real_commits; > + s->first_displayed_entry = TAILQ_FIRST(&s->real_commits.head); > + s->selected_entry = s->first_displayed_entry; > + s->selected = 0; > + s->limit_view = 0; > + > + return NULL; > + } > + > + if (regcomp(&s->limit_regex, pattern, REG_EXTENDED | REG_NEWLINE)) > + return NULL; > + > + s->limit_view = 1; > + > + /* Clear the screen while loading limit view */ > + s->first_displayed_entry = NULL; > + s->last_displayed_entry = NULL; > + s->selected_entry = NULL; > + s->commits = &s->limit_commits; > + > + /* Prepare limit queue for new search */ > + free_commits(&s->limit_commits); > + s->limit_commits.ncommits = 0; > + > + /* First process commits, which are in queue already */ > + TAILQ_FOREACH(entry, &s->real_commits.head, entry) { > + int have_match = 0; > + > + err = match_commit(&have_match, entry->id, > + entry->commit, &s->limit_regex); > + if (err) > + return err; > + > + if (have_match) { > + struct commit_queue_entry *matched; > + struct got_object_id *newid; > + > + matched = malloc(sizeof(*matched)); > + if (matched == NULL) > + return got_error_from_errno("malloc"); > + > + newid = calloc(1, sizeof(*newid)); > + if (newid == NULL) { > + free(matched); > + return got_error_from_errno("calloc"); > + } > + matched->id = newid; > + memcpy(matched->id->sha1, entry->id->sha1, > + SHA1_DIGEST_LENGTH); > + matched->commit = got_copy_commit(entry->commit); Please check the result of got_copy_commit(). > + matched->idx = s->limit_commits.ncommits; > + TAILQ_INSERT_TAIL(&s->limit_commits.head, > + matched, entry); > + s->limit_commits.ncommits++; > + } > + > + } > + > + /* Second process all the commits, until we fill the screen */ > + if (s->limit_commits.ncommits < view->nlines - 1 && > + !s->thread_args.log_complete) { > + s->thread_args.commits_needed += > + view->nlines - s->limit_commits.ncommits - 1; > + err = trigger_log_thread(view, 1); > + if (err) > + return err; > + } > + > + s->first_displayed_entry = TAILQ_FIRST(&s->commits->head); > + s->selected_entry = TAILQ_FIRST(&s->commits->head); > + s->selected = 0; > + > + return NULL; > +} > + > +static const struct got_error * > search_start_log_view(struct tog_view *view) > { > struct tog_log_view_state *s = &view->state.log; > @@ -3023,9 +3218,14 @@ open_log_view(struct tog_view *view, struct got_object > } > > /* The commit queue only contains commits being displayed. */ > - TAILQ_INIT(&s->commits.head); > - s->commits.ncommits = 0; > + TAILQ_INIT(&s->real_commits.head); > + s->real_commits.ncommits = 0; > + s->commits = &s->real_commits; > > + TAILQ_INIT(&s->limit_commits.head); > + s->limit_view = 0; > + s->limit_commits.ncommits = 0; > + > s->repo = repo; > if (head_ref_name) { > s->head_ref_name = strdup(head_ref_name); > @@ -3099,7 +3299,8 @@ open_log_view(struct tog_view *view, struct got_object > > s->thread_args.commits_needed = view->nlines; > s->thread_args.graph = thread_graph; > - s->thread_args.commits = &s->commits; > + s->thread_args.real_commits = &s->real_commits; > + s->thread_args.limit_commits = &s->limit_commits; > s->thread_args.in_repo_path = s->in_repo_path; > s->thread_args.start_id = s->start_id; > s->thread_args.repo = thread_repo; > @@ -3110,6 +3311,9 @@ open_log_view(struct tog_view *view, struct got_object > s->thread_args.searching = &view->searching; > s->thread_args.search_next_done = &view->search_next_done; > s->thread_args.regex = &view->regex; > + s->thread_args.limiting = &s->limit_view; > + s->thread_args.limit_regex = &s->limit_regex; > + s->thread_args.limit_commits = &s->limit_commits; > done: > if (err) > close_log_view(view); > @@ -3142,19 +3346,19 @@ log_move_cursor_up(struct tog_view *view, int page, in > { > struct tog_log_view_state *s = &view->state.log; > > + if (s->first_displayed_entry == NULL) > + return; > if (s->selected_entry->idx == 0) > view->count = 0; > - if (s->first_displayed_entry == NULL) > - return; > > - if ((page && TAILQ_FIRST(&s->commits.head) == s->first_displayed_entry) > + if ((page && TAILQ_FIRST(&s->commits->head) == s->first_displayed_entry) > || home) > s->selected = home ? 0 : MAX(0, s->selected - page - 1); > > if (!page && !home && s->selected > 0) > --s->selected; > else > - log_scroll_up(s, home ? s->commits.ncommits : MAX(page, 1)); > + log_scroll_up(s, home ? s->commits->ncommits : MAX(page, 1)); > > select_commit(s); > return; > @@ -3167,15 +3371,18 @@ log_move_cursor_down(struct tog_view *view, int page) > const struct got_error *err = NULL; > int eos = view->nlines - 2; > > + if (s->first_displayed_entry == NULL) > + return NULL; > + > if (s->thread_args.log_complete && > - s->selected_entry->idx >= s->commits.ncommits - 1) > + s->selected_entry->idx >= s->commits->ncommits - 1) > return NULL; > > if (view_is_hsplit_top(view)) > --eos; /* border consumes the last line */ > > if (!page) { > - if (s->selected < MIN(eos, s->commits.ncommits - 1)) > + if (s->selected < MIN(eos, s->commits->ncommits - 1)) > ++s->selected; > else > err = log_scroll_down(view, 1); > @@ -3184,7 +3391,7 @@ log_move_cursor_down(struct tog_view *view, int page) > int n; > > s->selected = 0; > - entry = TAILQ_LAST(&s->commits.head, commit_queue_head); > + entry = TAILQ_LAST(&s->commits->head, commit_queue_head); > s->last_displayed_entry = entry; > for (n = 0; n <= eos; n++) { > if (entry == NULL) > @@ -3195,10 +3402,10 @@ log_move_cursor_down(struct tog_view *view, int page) > if (n > 0) > s->selected = n - 1; > } else { > - if (s->last_displayed_entry->idx == s->commits.ncommits - 1 && > + if (s->last_displayed_entry->idx == s->commits->ncommits - 1 && > s->thread_args.log_complete) > s->selected += MIN(page, > - s->commits.ncommits - s->selected_entry->idx - 1); > + s->commits->ncommits - s->selected_entry->idx - 1); > else > err = log_scroll_down(view, page); > } > @@ -3218,7 +3425,7 @@ log_move_cursor_down(struct tog_view *view, int page) > select_commit(s); > > if (s->thread_args.log_complete && > - s->selected_entry->idx == s->commits.ncommits - 1) > + s->selected_entry->idx == s->commits->ncommits - 1) > view->count = 0; > > return NULL; > @@ -3314,7 +3521,7 @@ input_log_view(struct tog_view **new_view, struct tog_ > if (ch == CTRL('g') || ch == KEY_BACKSPACE) > s->thread_args.load_all = 0; > else if (s->thread_args.log_complete) { > - err = log_move_cursor_down(view, s->commits.ncommits); > + err = log_move_cursor_down(view, s->commits->ncommits); > s->thread_args.load_all = 0; > } > if (err) > @@ -3329,6 +3536,9 @@ input_log_view(struct tog_view **new_view, struct tog_ > return log_goto_line(view, eos); > > switch (ch) { > + case '&': > + err = limit_log_view(view); > + break; > case 'q': > s->quit = 1; > break; > @@ -3391,7 +3601,7 @@ input_log_view(struct tog_view **new_view, struct tog_ > s->thread_args.load_all = 1; > if (!s->thread_args.log_complete) > return trigger_log_thread(view, 0); > - err = log_move_cursor_down(view, s->commits.ncommits); > + err = log_move_cursor_down(view, s->commits->ncommits); > s->thread_args.load_all = 0; > break; > } > @@ -3408,13 +3618,13 @@ input_log_view(struct tog_view **new_view, struct tog_ > case KEY_RESIZE: > if (s->selected > view->nlines - 2) > s->selected = view->nlines - 2; > - if (s->selected > s->commits.ncommits - 1) > - s->selected = s->commits.ncommits - 1; > + if (s->selected > s->commits->ncommits - 1) > + s->selected = s->commits->ncommits - 1; > select_commit(s); > - if (s->commits.ncommits < view->nlines - 1 && > + if (s->commits->ncommits < view->nlines - 1 && > !s->thread_args.log_complete) { > s->thread_args.commits_needed += (view->nlines - 1) - > - s->commits.ncommits; > + s->commits->ncommits; > err = trigger_log_thread(view, 1); > } > break; > @@ -3484,7 +3694,8 @@ input_log_view(struct tog_view **new_view, struct tog_ > s->start_id, s->repo, NULL, NULL); > if (err) > return err; > - free_commits(&s->commits); > + free_commits(&s->real_commits); > + free_commits(&s->limit_commits); > s->first_displayed_entry = NULL; > s->last_displayed_entry = NULL; > s->selected_entry = NULL; > -- Mark Jamsek <fnc.bsdbox.org> GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
tog: limit feature