On Fri, Nov 29, 2019 at 11:00:01PM +0100, Klemens Nanni wrote:
> On Fri, Nov 29, 2019 at 02:28:14PM -0700, Stefan Sperling wrote:
> > In my mind, Git's use of term 'grep' for searching log messages seems
> > misplaced. I believe we should reserve the term "grep" for an operation
> > which searches content of blobs in a given commit/tree, as an analogy
> > to how regular UNIX grep(1) searches files on disk.
> > 
> > So I would prefer the term "search" over "grep", and option -s over -g.
> > And all variable and function names should also be adapted to use a
> > search_ prefix instead of a grep_ prefix.
> Sure, fine with me.

Great, thanks!

> I just return immediately, `done' is too much as neither the regex must
> be freed nor the commit graph must be closed.
> 
> OK?

Yes, this looks fine.

> -----------------------------------------------
> commit 6841bf1343d4d3bbafbc1db614429635309c20d4
> from: Klemens Nanni <kn@openbsd.org>
> date: Fri Nov 29 21:54:03 2019 UTC
>  
>  log: Implement -s search-pattern
>  
>  match_logmsg() is copied from tog's match_commit().
>  
> diff c48aed8c5e0fe5a2ebc85da3f009d43d33a94b52 ed9efb3599fbb99d58b35bc58e27b9fb2b50242f
> blob - 9c0c3772a7c0a876c6e1fc5ae868fe928503cb90
> blob + 59c41698313a510ea5d7712e81349bf7d7bb7d6d
> --- got/got.1
> +++ got/got.1
> @@ -314,7 +314,7 @@ in a pattern.
>  .It Cm st
>  Short alias for
>  .Cm status .
> -.It Cm log Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl f Oc Oo Fl l Ar N Oc Oo Fl p Oc Oo Fl r Ar repository-path Oc Op Ar path
> +.It Cm log Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl f Oc Oo Fl l Ar N Oc Oo Fl p Oc Oo Fl g Ar search-pattern Oc Oo Fl r Ar repository-path Oc Op Ar path
>  Display history of a repository.
>  If a
>  .Ar path
> @@ -354,6 +354,12 @@ Display the patch of modifications made in each commit
>  If a
>  .Ar path
>  is specified, only show the patch of modifications at or within this path.
> +.It Fl g Ar search-pattern
> +If specified, show only commits which log message matches the extended
> +regular expression
> +.Ar search-pattern .
> +Regular expression syntax is documented in
> +.Xr re_format 7 .
>  .It Fl r Ar repository-path
>  Use the repository at the specified path.
>  If not specified, assume the repository is located at or above the current
> blob - 9f403407a6f1173c7f605a66f2f04a512323bd71
> blob + a563bd3577b7aafe51ef56016983954041267f18
> --- got/got.c
> +++ got/got.c
> @@ -34,6 +34,7 @@
>  #include <libgen.h>
>  #include <time.h>
>  #include <paths.h>
> +#include <regex.h>
>  
>  #include "got_version.h"
>  #include "got_error.h"
> @@ -1589,6 +1590,32 @@ get_datestr(time_t *time, char *datebuf)
>  	return s;
>  }
>  
> +static const struct got_error *
> +match_logmsg(int *have_match, struct got_object_id *id,
> +    struct got_commit_object *commit, regex_t *regex)
> +{
> +	const struct got_error *err = NULL;
> +	regmatch_t regmatch;
> +	char *id_str = NULL, *logmsg = NULL;
> +
> +	*have_match = 0;
> +
> +	err = got_object_id_str(&id_str, id);
> +	if (err)
> +		return err;
> +
> +	err = got_object_commit_get_logmsg(&logmsg, commit);
> +	if (err)
> +		goto done;
> +
> +	if (regexec(regex, logmsg, 1, &regmatch, 0) == 0)
> +		*have_match = 1;
> +done:
> +	free(id_str);
> +	free(logmsg);
> +	return err;
> +}
> +
>  #define GOT_COMMIT_SEP_STR "-----------------------------------------------\n"
>  
>  static const struct got_error *
> @@ -1705,12 +1732,18 @@ print_commit(struct got_commit_object *commit, struct 
>  
>  static const struct got_error *
>  print_commits(struct got_object_id *root_id, struct got_repository *repo,
> -    char *path, int show_patch, int diff_context, int limit,
> -    int first_parent_traversal, struct got_reflist_head *refs)
> +    char *path, int show_patch, char *search_pattern, int diff_context,
> +    int limit, int first_parent_traversal, struct got_reflist_head *refs)
>  {
>  	const struct got_error *err;
>  	struct got_commit_graph *graph;
> +	regex_t regex;
> +	int have_match;
>  
> +	if (search_pattern &&
> +	    regcomp(&regex, search_pattern, REG_EXTENDED | REG_NOSUB | REG_NEWLINE))
> +		return got_error_msg(GOT_ERR_REGEX, search_pattern);
> +
>  	err = got_commit_graph_open(&graph, root_id, path,
>  	    first_parent_traversal, repo);
>  	if (err)
> @@ -1747,6 +1780,19 @@ print_commits(struct got_object_id *root_id, struct go
>  		err = got_object_open_as_commit(&commit, repo, id);
>  		if (err)
>  			break;
> +
> +		if (search_pattern) {
> +			err = match_logmsg(&have_match, id, commit, &regex);
> +			if (err) {
> +				got_object_commit_close(commit);
> +				break;
> +			}
> +			if (have_match == 0) {
> +				got_object_commit_close(commit);
> +				continue;
> +			}
> +		}
> +
>  		err = print_commit(commit, id, repo, path, show_patch,
>  		    diff_context, refs);
>  		got_object_commit_close(commit);
> @@ -1754,6 +1800,8 @@ print_commits(struct got_object_id *root_id, struct go
>  			break;
>  	}
>  done:
> +	if (search_pattern)
> +		regfree(&regex);
>  	got_commit_graph_close(graph);
>  	return err;
>  }
> @@ -1762,7 +1810,7 @@ __dead static void
>  usage_log(void)
>  {
>  	fprintf(stderr, "usage: %s log [-c commit] [-C number] [-f] [ -l N ] [-p] "
> -	    "[-r repository-path] [path]\n", getprogname());
> +	    "[-s search-pattern] [-r repository-path] [path]\n", getprogname());
>  	exit(1);
>  }
>  
> @@ -1791,7 +1839,7 @@ cmd_log(int argc, char *argv[])
>  	struct got_commit_object *commit = NULL;
>  	struct got_object_id *id = NULL;
>  	char *repo_path = NULL, *path = NULL, *cwd = NULL, *in_repo_path = NULL;
> -	char *start_commit = NULL;
> +	char *start_commit = NULL, *search_pattern = NULL;
>  	int diff_context = 3, ch;
>  	int show_patch = 0, limit = 0, first_parent_traversal = 0;
>  	const char *errstr;
> @@ -1808,7 +1856,7 @@ cmd_log(int argc, char *argv[])
>  
>  	limit = get_default_log_limit();
>  
> -	while ((ch = getopt(argc, argv, "b:pc:C:l:fr:")) != -1) {
> +	while ((ch = getopt(argc, argv, "b:pc:C:l:fr:s:")) != -1) {
>  		switch (ch) {
>  		case 'p':
>  			show_patch = 1;
> @@ -1837,6 +1885,9 @@ cmd_log(int argc, char *argv[])
>  				    optarg);
>  			got_path_strip_trailing_slashes(repo_path);
>  			break;
> +		case 's':
> +			search_pattern = optarg;
> +			break;
>  		default:
>  			usage_log();
>  			/* NOTREACHED */
> @@ -1982,7 +2033,7 @@ cmd_log(int argc, char *argv[])
>  	if (error)
>  		goto done;
>  
> -	error = print_commits(id, repo, path, show_patch,
> +	error = print_commits(id, repo, path, show_patch, search_pattern,
>  	    diff_context, limit, first_parent_traversal, &refs);
>  done:
>  	free(path);
> 
>