"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: tog: limit feature
To:
Mikhail <mp39590@gmail.com>
Cc:
Mark Jamsek <mark@jamsek.com>, gameoftrees@openbsd.org
Date:
Sun, 11 Sep 2022 12:18:04 +0200

Download raw body.

Thread
Hello,

On 2022/09/03 09:49:13 +0300, Mikhail <mp39590@gmail.com> wrote:
> On Fri, Sep 02, 2022 at 07:58:20PM +1000, Mark Jamsek wrote:
> > 
> > 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.
>  
> New version:

First of all, sorry for all this back and forward.  I love the feature
introduced by the diff but I haven't had the time to properly review
it before (was side-tracked by other things) and this thread has
already dragged on too much.  Sorry for this, and thanks for
continuing working on it!

There is however a couple of points that i'd like to address before
this is committed.  Some comments inline and an updated diff attached
below.

> diff refs/heads/main refs/heads/limit7
> commit - 4ba2e95571e317867b5ed45cb6c8580a33627500
> commit + 04da6d0f4abe7a084178339eb25eee03c63271cb
> blob - 4e49d36a9dfb5392b25c430b1fa90b30c898c3ed
> blob + 87d264cf61b2c0de8300c6e5a90bab9d69b0142c
> --- 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 *);

more than "copy" the existing API are using "dup", so I'd personally
go with `got_object_commit_dup'.

> blob - 9fcd4fa3978b2e29657e3fb07c4d6af41f6427cd
> blob + 0a2d51ce9cf9d82bf4b35dfe3d1f6d3eb545c19a
> --- lib/object.c
> +++ lib/object.c
> @@ -2352,3 +2352,73 @@ 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;
> [...]

There's no need to duplicate the commit like this.  The commits
objects are under reference counted so we can just increment the
reference number and return the original pointer.

if tog' threading should become an issue we could fix it there by
using some mutexes or atomic inc/dec on the reference counter.  it
wasn't a problem until now however, so I doubt this change requires
it.

bumping the reference couter also has the advantage of reducing the
amount of extra memory needed for the separate queue.

> +}
> blob - 65405972315ce384c286567d1765d330d8d9a13f
> blob + 7f5838ca56a87f272be0f2ddb8af1b6feb75c238
> --- tog/tog.1
> +++ tog/tog.1
> @@ -226,6 +226,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.

nitpick: what if we move this after the /-key description and just say
that has the same behavior but hides the non-matching commits?  We
could drop some copy-paste then.

>  .El
>  .Pp
>  The options for
> blob - d3fe18a13136b64b9e9ba153ae5a2deda0482b9c
> blob + ccbc818b939ef215841e7287e6be8e4e3b01eb63
> --- 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,69 @@ 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));

Two minor nits here:

 1. personally i prefer to use calloc in this case, should we forget
    to initialize something it would get zero'ed at least.

 2. instead of sizeof(type) you can also use sizeof(variable) which
    has the benefit of being shorter (no line wrap ;-) and being fine
    if we need to change `matched' type' but retain the struct
    commit_queue_entry.

> +				if (matched == NULL)
> +					return got_error_from_errno("malloc");

I'm also not sure of the error handling in this function.  If one of
these function fails we exit the loop, leaking the items processed so
far.  I'm not fixing this in the diff below and can be sorted out
later in-tree after this gets committed.

> +				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);
> +				if (matched->commit == NULL) {
> +					free(matched);
> +					free(newid);
> +					return got_error_from_errno(
> +					    "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 +2332,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 +2340,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 +2351,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";

what about "no 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 : ""),

not an issue with your diff, but for the mental sanity of who reads
this I'd prefer if the outmost ?: ternary wasn't wrapped.

I mean:

		if (...
		    search_str ? search_str : (refs_str ? refs_str : ""),
		    ... == -1)

and maybe consider adding a temp. variable there as a follow-up
commit.  (it's an existing issue, not introduced by your diff.)

> +		    limit_str ? limit_str : "") == -1) {
>  			err = got_error_from_errno("asprintf");
>  			goto done;
>  		}
> @@ -2419,7 +2485,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 +2570,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 +2816,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 +2831,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 +2942,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 +2953,139 @@ 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.
> + */

I took the liberty to reflow this comment so it fits more nicely in
the 80 columns :)

(i think that M-q on Emacs tries to fit the text under 72 chars which
I personally find nice for comments.)

>  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: ");

I'd use "&/" as less(1) does.

> +	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);
> +			if (matched->commit == NULL) {
> +				free(matched);
> +				free(newid);
> +				return got_error_from_errno("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 +3231,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 +3312,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 +3324,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 +3359,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 +3384,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 +3404,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 +3415,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 +3438,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 +3534,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 +3549,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 +3614,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 +3631,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 +3707,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;


The rest looks ok for me.  Here's a diff that addresses the mentioned
points.

As a follow-up i think that upon C-l during & we should free also the
original list of commits, not only the matched queue.

diff /home/op/w/got
commit - d9787ed86ecec0bda7a570181d86c44ba80bd583
path + /home/op/w/got
blob - 4e49d36a9dfb5392b25c430b1fa90b30c898c3ed
file + include/got_object.h
--- include/got_object.h
+++ include/got_object.h
@@ -352,3 +352,6 @@ 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);
+
+const struct got_error *got_object_commit_dup(struct got_commit_object **,
+    struct got_repository *, struct got_commit_object *);
blob - 27d10eb82fe55e97ef790f60755120c3b6381917
file + lib/object.c
--- lib/object.c
+++ lib/object.c
@@ -2353,3 +2353,12 @@ done:
 	free(path_packfile);
 	return err;
 }
+
+const struct got_error *
+got_object_commit_dup(struct got_commit_object **ret,
+    struct got_repository *repo, struct got_commit_object *commit)
+{
+	*ret = commit;
+	commit->refcnt++;
+	return NULL;
+}
blob - 65405972315ce384c286567d1765d330d8d9a13f
file + tog/tog.1
--- tog/tog.1
+++ tog/tog.1
@@ -189,6 +189,10 @@ against a commit's author name, committer name, log me
 commit ID SHA1 hash.
 Regular expression syntax is documented in
 .Xr re_format 7 .
+.It Cm &
+Like
+.Cm /
+but hides the non-matching commits.
 .It Cm n
 Find the Nth next commit which matches the current search pattern (default: 1).
 .br
blob - 7dd9d7c85c373e47b6c6c9bd21a4109d8af43d19
file + tog/tog.c
--- 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);
 	}
 
@@ -2174,6 +2182,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,
@@ -2197,17 +2206,64 @@ 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 = calloc(1, 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);
+
+				err = got_object_commit_dup(&matched->commit,
+				    a->repo, entry->commit);
+				if (err)
+					break;
+
+				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;
 		}
 
@@ -2278,7 +2334,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");
@@ -2286,6 +2342,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)
@@ -2296,10 +2353,13 @@ draw_commits(struct tog_view *view)
 				search_str = "searching...";
 		}
 
-		if (asprintf(&ncommits_str, " [%d/%d] %s",
-		    entry ? entry->idx + 1 : 0, s->commits.ncommits,
-		    search_str ? search_str :
-		    (refs_str ? refs_str : "")) == -1) {
+		if (s->limit_view && s->commits->ncommits == 0)
+			limit_str = "no 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 : ""),
+		    limit_str ? limit_str : "") == -1) {
 			err = got_error_from_errno("asprintf");
 			goto done;
 		}
@@ -2424,7 +2484,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;
 
@@ -2509,13 +2569,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;
@@ -2755,8 +2815,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) {
@@ -2765,10 +2830,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);
@@ -2872,7 +2941,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);
@@ -2882,7 +2952,141 @@ 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, "&/");
+	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);
+
+			err = got_object_commit_dup(&matched->commit,
+			    s->repo, entry->commit);
+			if (err) {
+				free(matched);
+				free(newid);
+				return err;
+			}
+
+			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;
@@ -3028,9 +3232,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);
@@ -3104,7 +3313,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;
@@ -3115,6 +3325,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);
@@ -3147,19 +3360,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;
@@ -3172,15 +3385,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);
@@ -3189,7 +3405,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)
@@ -3200,10 +3416,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);
 	}
@@ -3223,7 +3439,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;
@@ -3319,7 +3535,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)
@@ -3334,6 +3550,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;
@@ -3396,7 +3615,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;
 	}
@@ -3413,13 +3632,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;
@@ -3489,7 +3708,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;