From: Evan Silberman Subject: Re: [patch] add a one-line-per-commit output mode to log To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Tue, 07 Jun 2022 21:43:27 -0700 Stefan Sperling wrote: > On Sat, Jun 04, 2022 at 04:05:18PM -0700, Evan Silberman wrote: > > -.It Cm log Oo Fl b Oc Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl l Ar N Oc Oo Fl p Oc Oo Fl P Oc Oo Fl s Ar search-pattern Oc Oo Fl r Ar repository-path Oc Oo Fl R Oc Oo Fl x Ar commit Oc Op Ar path > > +.It Cm log Oo Fl b Oc Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl l Ar N Oc Oo Fl pP | s Oc Oo Fl S Ar search-pattern Oc Oo Fl r Ar repository-path Oc Oo Fl R Oc Oo Fl x Ar commit Oc Op Ar path > > Showing -p -P -s instead of [ -pP | -s ] would be more consistent with > other parts of the man page, and with other existing usage() strings. Check. > So far, we don't represent exclusivity of options in the synopsis line. > I suspect doing so consistently would become complicated very quickly > for some of our commands. Synopsis and usage strings can just list all > valid options. Details are documented in the option description text. > > > static const struct got_error * > > +print_commit_oneline(struct got_commit_object *commit, struct got_object_id *id) > > +{ > > + const struct got_error *err = NULL; > > + char *id_str, *s, *nl, *logmsg0, *logmsg = NULL; > > + > > + err = got_object_id_str(&id_str, id); > > + if (err) > > + return err; > > + > > + err = got_object_commit_get_logmsg(&logmsg0, commit); > > + if (err) > > + goto done; > > + > > + s = logmsg0; > > + while (isspace((unsigned char)s[0])) > > + s++; > > + > > + logmsg = strdup(s); > > There should be no need for strdup(). logmsg0 is already a copy of the > commit's log message string so writing a '\0' to the logmsg0 buffer is fine. > You forgot to free(logmsg), but this won't be needed if we decide to simply > drop the strdup() call. An artifact of my copy-paste-iterate-driven development process. My skill level with C string handling is at the "guessing" stage. > > + if (logmsg == NULL) { > > + err = got_error_from_errno("strdup"); > > + goto done; > > + } > > + > > + nl = strchr(logmsg, '\n'); > > + if (nl) { > > + *nl = '\0'; > > + } > > + > > + printf("%.7s %s\n", id_str, logmsg); > > The current output format of log -s is quite sparse. It might be a good > idea to extend it. For example, it could include committer date and name, > like the default log display shown by 'tog log'. But this can be tweaked > later. This suggests still further interesting questions about the UI design space. git log --pretty has 9 predefined formats, all orthogonal to the --decorate flag, plus a formatter language with dozens of tokens accessible as well as alignment and color output directives. I'd take it as a given that got doesn't want to support _quite_ that much but certainly my contribution here doesn't seem like the end of the line either. (fwiw this patch makes -s behave basically the same as git log --oneline --no-decorate.) I think you'd want to make a design decision about what your UI should be for this before even more options and combinations of options get into the mix. Fresh patch attached. Evan ----------------------------------------------- commit 739f42edd1683838b8ce3e27eee12ed3078fed51 (log-oneline) from: Evan Silberman date: Wed Jun 8 04:27:45 2022 UTC Add one-line output mode to got log with -s flag -s for short. Moves log search to -S. diff b801cfa6010340244886ed8bcc5895a2831497e7 d9a8a2a59e0d34f20aaa79c5101e0e9909f9ca42 blob - b91cfb2cb4963721a9a86be9c2cdb397bc1a9278 blob + 44f372d9146e3367bcbd15b4681c57b1b340faef --- got/got.1 +++ got/got.1 @@ -756,7 +756,7 @@ does not support negated ignore patterns prefixed with and gives no special significance to the location of path component separators, .Dq / , in a pattern. -.It Cm log Oo Fl b Oc Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl l Ar N Oc Oo Fl p Oc Oo Fl P Oc Oo Fl s Ar search-pattern Oc Oo Fl r Ar repository-path Oc Oo Fl R Oc Oo Fl x Ar commit Oc Op Ar path +.It Cm log Oo Fl b Oc Oo Fl c Ar commit Oc Oo Fl C Ar number Oc Oo Fl l Ar N Oc Oo Fl p Oc Oo Fl P Oc Oo Fl s Oc Oo Fl S Ar search-pattern Oc Oo Fl r Ar repository-path Oc Oo Fl R Oc Oo Fl x Ar commit Oc Op Ar path Display history of a repository. If a .Ar path @@ -802,6 +802,9 @@ 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. +Cannot be used with the +.Fl s +option. .It Fl P Display the list of file paths changed in each commit, using the following status codes: @@ -811,7 +814,20 @@ status codes: .It A Ta new file was added .It m Ta modified file modes (executable bit only) .El -.It Fl s Ar search-pattern +Cannot be used with the +.Fl s +option. +.It Fl s +Display a short one-line summary of each commit, instead of the default +history format. +The one-line format prints the beginning of the SHA1 hash of each commit, +followed by a space and the first line of the log message. +Cannot be used together with the +.Fl p +or +.Fl P +option. +.It Fl S Ar search-pattern If specified, show only commits with a log message matched by the extended regular expression .Ar search-pattern . blob - d88f23ce5b63a9d80862b5fac05c212d054a0b74 blob + 473318fc731afd091ffa6dcf1c0a27f6c47cda82 --- got/got.c +++ got/got.c @@ -3841,6 +3841,39 @@ build_refs_str(char **refs_str, struct got_reflist_hea } static const struct got_error * +print_commit_oneline(struct got_commit_object *commit, struct got_object_id *id) +{ + const struct got_error *err = NULL; + char *id_str, *s, *nl, *logmsg0; + + err = got_object_id_str(&id_str, id); + if (err) + return err; + + err = got_object_commit_get_logmsg(&logmsg0, commit); + if (err) + goto done; + + s = logmsg0; + while (isspace((unsigned char)s[0])) + s++; + + nl = strchr(s, '\n'); + if (nl) { + *nl = '\0'; + } + + printf("%.7s %s\n", id_str, s); + + if (fflush(stdout) != 0 && err == NULL) + err = got_error_from_errno("fflush"); +done: + free(id_str); + free(logmsg0); + return err; +} + +static const struct got_error * print_commit(struct got_commit_object *commit, struct got_object_id *id, struct got_repository *repo, const char *path, struct got_pathlist_head *changed_paths, int show_patch, @@ -3941,7 +3974,7 @@ print_commits(struct got_object_id *root_id, struct go struct got_repository *repo, const char *path, int show_changed_paths, int show_patch, const char *search_pattern, int diff_context, int limit, int log_branches, int reverse_display_order, - struct got_reflist_object_id_map *refs_idmap) + struct got_reflist_object_id_map *refs_idmap, int one_line) { const struct got_error *err; struct got_commit_graph *graph; @@ -4020,9 +4053,12 @@ print_commits(struct got_object_id *root_id, struct go STAILQ_INSERT_HEAD(&reversed_commits, qid, entry); got_object_commit_close(commit); } else { - err = print_commit(commit, id, repo, path, - show_changed_paths ? &changed_paths : NULL, - show_patch, diff_context, refs_idmap, NULL); + if (one_line) + err = print_commit_oneline(commit, id); + else + err = print_commit(commit, id, repo, path, + show_changed_paths ? &changed_paths : NULL, + show_patch, diff_context, refs_idmap, NULL); got_object_commit_close(commit); if (err) break; @@ -4049,9 +4085,12 @@ print_commits(struct got_object_id *root_id, struct go if (err) break; } - err = print_commit(commit, &qid->id, repo, path, - show_changed_paths ? &changed_paths : NULL, - show_patch, diff_context, refs_idmap, NULL); + if (one_line) + err = print_commit_oneline(commit, &qid->id); + else + err = print_commit(commit, &qid->id, repo, path, + show_changed_paths ? &changed_paths : NULL, + show_patch, diff_context, refs_idmap, NULL); got_object_commit_close(commit); if (err) break; @@ -4082,9 +4121,9 @@ done: __dead static void usage_log(void) { - fprintf(stderr, "usage: %s log [-b] [-c commit] [-C number] [ -l N ] " - "[-p] [-P] [-x commit] [-s search-pattern] [-r repository-path] " - "[-R] [path]\n", getprogname()); + fprintf(stderr, "usage: %s log [-b] [-p] [-P] [-s] [-c commit] " + "[-C number] [ -l N ] [-x commit] [-S search-pattern] " + "[-r repository-path] [-R] [path]\n", getprogname()); exit(1); } @@ -4116,7 +4155,7 @@ cmd_log(int argc, char *argv[]) const char *search_pattern = NULL; int diff_context = -1, ch; int show_changed_paths = 0, show_patch = 0, limit = 0, log_branches = 0; - int reverse_display_order = 0; + int reverse_display_order = 0, one_line = 0; const char *errstr; struct got_reflist_head refs; struct got_reflist_object_id_map *refs_idmap = NULL; @@ -4132,7 +4171,7 @@ cmd_log(int argc, char *argv[]) limit = get_default_log_limit(); - while ((ch = getopt(argc, argv, "bpPc:C:l:r:Rs:x:")) != -1) { + while ((ch = getopt(argc, argv, "bpPc:C:l:r:RsS:x:")) != -1) { switch (ch) { case 'p': show_patch = 1; @@ -4170,6 +4209,9 @@ cmd_log(int argc, char *argv[]) reverse_display_order = 1; break; case 's': + one_line = 1; + break; + case 'S': search_pattern = optarg; break; case 'x': @@ -4189,6 +4231,9 @@ cmd_log(int argc, char *argv[]) else if (!show_patch) errx(1, "-C requires -p"); + if (one_line && (show_patch || show_changed_paths)) + errx(1, "cannot use -s with -p or -P"); + cwd = getcwd(NULL, 0); if (cwd == NULL) { error = got_error_from_errno("getcwd"); @@ -4307,7 +4352,7 @@ cmd_log(int argc, char *argv[]) error = print_commits(start_id, end_id, repo, path ? path : "", show_changed_paths, show_patch, search_pattern, diff_context, - limit, log_branches, reverse_display_order, refs_idmap); + limit, log_branches, reverse_display_order, refs_idmap, one_line); done: free(path); free(repo_path); blob - f13bd9029079658e7f658b35c11dc4ca7a6f70f7 blob + 41cee9ee65ad11031857077637457e6954320463 --- regress/cmdline/log.sh +++ regress/cmdline/log.sh @@ -297,6 +297,40 @@ test_log_limit() { test_done "$testroot" "0" } +test_log_oneline() { + local testroot=`test_init log_oneline` + local commit_id0=`git_show_head $testroot/repo` + got checkout $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" "$ret" + return 1 + fi + + echo "modified alpha" > $testroot/wt/alpha + (cd $testroot/wt && got commit -m "test oneline +no" > /dev/null) + local commit_id1=`git_show_head $testroot/repo` + + echo "modified beta" > $testroot/wt/beta + (cd $testroot/wt && got commit -m " test oneline +no" > /dev/null) + local commit_id2=`git_show_head $testroot/repo` + + printf "%.7s test oneline\n" $commit_id2 > $testroot/stdout.expected + printf "%.7s test oneline\n" $commit_id1 >> $testroot/stdout.expected + + (cd $testroot/repo && got log -s | head -n 2 > $testroot/stdout) + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + test_done "$testroot" "0" +} + test_log_patch_added_file() { local testroot=`test_init log_patch_added_file` local commit_id0=`git_show_head $testroot/repo` @@ -587,7 +621,7 @@ test_log_reverse_display() { # commit matching with -s applies before -R echo "commit $commit_id1" > $testroot/stdout.expected echo "commit $commit_id2" >> $testroot/stdout.expected - (cd $testroot/wt && got log -R -s 'commit[12]' | \ + (cd $testroot/wt && got log -R -S 'commit[12]' | \ grep ^commit > $testroot/stdout) cmp -s $testroot/stdout.expected $testroot/stdout ret=$? @@ -815,6 +849,7 @@ run_test test_log_in_worktree run_test test_log_in_worktree_with_path_prefix run_test test_log_tag run_test test_log_limit +run_test test_log_oneline run_test test_log_patch_added_file run_test test_log_nonexistent_path run_test test_log_end_at_commit