From: Omar Polo Subject: Re: make got log -s show date and branches/tags To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Mon, 13 Jun 2022 11:22:22 +0200 Stefan Sperling wrote: > Make got log -s display commit timestamps in YYYY-MM-DD format, > and display reference names instead of an abbreviated hash if > the reference is a local branch or a tag. > > Before: > > $ got log -s -l5 > 6ec249e remove outdated and overly-specific documentation of got log -s output > b58982a adjust expected test output after got log -s changes > 92d06b6 make got log -s show committer date > 598bd2c make got log -s display local reference names > 3ef807e reword description of how got log -p and -P interact with got log -S > > After: > > $ got log -s -l5 > 2022-06-11 log-s remove outdated and overly-specific documentation of got log -s output > 2022-06-11 b58982a adjust expected test output after got log -s changes > 2022-06-11 92d06b6 make got log -s show committer date > 2022-06-11 598bd2c make got log -s display local reference names > 2022-06-08 main reword description of how got log -p and -P interact with got log -S > > I believe showing the date is important because commit hashes by themselves > provide no indication of timeline at all. Dates make log -s a lot more useful. > > Showing branch names and tags is more readable than always showing > hashes. With longer branch names the log message gets indented a > bit further, but that is no big deal. I chose to omit remote refs > because they cause more indentation and are not always interesting. > > I considered adding author info as well but decided against it. > Lines printed by log -s are already too long on small terminals. > > I also considered truncating the log message to terminal size but this > is impossible to do correctly without converting to wide characters to > find split-points in UTF-8 byte strings. Since tog already covers this > for people who care about such optics, log -s can stay simple. Yes, it'd be nice but it's not easy. there's always less -S anyway :) > Instead of documenting the output changes, just remove overly specific > documentation so we don't have to worry about keeping it up-to-date. > > ok? Love the idea. OK op@ nit below > diff refs/heads/main refs/heads/log-s > blob - 7825117a1854a2737ac5b841f83716ac3749d20a > blob + 769726c3e688f4817f2a83df5beae11d34ea8948 > --- got/got.1 > +++ got/got.1 > @@ -821,8 +821,6 @@ 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 > blob - 28ba9a46f202c773426d431a909c5dec6304d3ca > blob + 4e1ffc4ac48138e3b509a39589e7d444b9957ab8 > --- got/got.c > +++ got/got.c > @@ -3814,7 +3814,8 @@ done: > > static const struct got_error* > build_refs_str(char **refs_str, struct got_reflist_head *refs, > - struct got_object_id *id, struct got_repository *repo) > + struct got_object_id *id, struct got_repository *repo, > + int local_only) > { > static const struct got_error *err = NULL; > struct got_reflist_entry *re; > @@ -3838,6 +3839,8 @@ build_refs_str(char **refs_str, struct got_reflist_hea > if (strncmp(name, "heads/", 6) == 0) > name += 6; > if (strncmp(name, "remotes/", 8) == 0) { > + if (local_only) > + continue; > name += 8; > s = strstr(name, "/" GOT_REF_HEAD); > if (s != NULL && s[strlen(s)] == '\0') > @@ -3880,15 +3883,40 @@ 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) > +print_commit_oneline(struct got_commit_object *commit, struct got_object_id *id, > + struct got_repository *repo, struct got_reflist_object_id_map *refs_idmap) > { > const struct got_error *err = NULL; > - char *id_str, *s, *nl, *logmsg0; > + char *ref_str = NULL, *id_str = NULL, *logmsg0 = NULL; > + char *comma, *s, *nl; > + struct got_reflist_head *refs; > + char datebuf[12]; /* YYYY-MM-DD + SPACE + NUL */ > + struct tm tm; > + time_t committer_time; > > - err = got_object_id_str(&id_str, id); > - if (err) > - return err; > + refs = got_reflist_object_id_map_lookup(refs_idmap, id); > + if (refs) { > + err = build_refs_str(&ref_str, refs, id, repo, 1); > + if (err) > + goto done; (this may become a return err.) > + /* Display the first matching ref only. */ > + if (ref_str && (comma = strchr(ref_str, ',')) != NULL) > + *comma = '\0'; > + } > + > + if (ref_str == NULL) { > + err = got_object_id_str(&id_str, id); > + if (err) > + return err; > + } > + > + committer_time = got_object_commit_get_committer_time(commit); > + if (gmtime_r(&committer_time, &tm) == NULL) > + return got_error_from_errno("gmtime_r"); > + if (strftime(datebuf, sizeof(datebuf), "%G-%m-%d ", &tm) == 0) > + return got_error(GOT_ERR_NO_SPACE); this two returns need to become goto done otherwise we're possibly leaking ref_str. (very unlikely but...) > err = got_object_commit_get_logmsg(&logmsg0, commit); > if (err) > goto done; > @@ -3902,12 +3930,16 @@ print_commit_oneline(struct got_commit_object *commit, > *nl = '\0'; > } > > - printf("%.7s %s\n", id_str, s); > + if (ref_str) > + printf("%s%-7s %s\n", datebuf, ref_str, s); > + else > + printf("%s%.7s %s\n", datebuf, id_str, s); > > if (fflush(stdout) != 0 && err == NULL) > err = got_error_from_errno("fflush"); > done: > free(id_str); > + free(ref_str); > free(logmsg0); > return err; > } > @@ -3934,7 +3966,7 @@ print_commit(struct got_commit_object *commit, struct > struct got_reflist_head *refs; > refs = got_reflist_object_id_map_lookup(refs_idmap, id); > if (refs) { > - err = build_refs_str(&refs_str, refs, id, repo); > + err = build_refs_str(&refs_str, refs, id, repo, 0); > if (err) > goto done; > } > @@ -4101,7 +4133,8 @@ print_commits(struct got_object_id *root_id, struct go > got_object_commit_close(commit); > } else { > if (one_line) > - err = print_commit_oneline(commit, id); > + err = print_commit_oneline(commit, id, > + repo, refs_idmap); > else > err = print_commit(commit, id, repo, path, > show_changed_paths ? &changed_paths : NULL, > @@ -4133,7 +4166,8 @@ print_commits(struct got_object_id *root_id, struct go > break; > } > if (one_line) > - err = print_commit_oneline(commit, &qid->id); > + err = print_commit_oneline(commit, &qid->id, > + repo, refs_idmap); > else > err = print_commit(commit, &qid->id, repo, path, > show_changed_paths ? &changed_paths : NULL, > @@ -9222,7 +9256,7 @@ print_backup_ref(const char *branch_name, const char * > > refs = got_reflist_object_id_map_lookup(refs_idmap, new_commit_id); > if (refs) { > - err = build_refs_str(&refs_str, refs, new_commit_id, repo); > + err = build_refs_str(&refs_str, refs, new_commit_id, repo, 0); > if (err) > goto done; > } > @@ -9262,7 +9296,7 @@ print_backup_ref(const char *branch_name, const char * > > refs = got_reflist_object_id_map_lookup(refs_idmap, yca_id); > if (refs) { > - err = build_refs_str(&refs_str, refs, yca_id, repo); > + err = build_refs_str(&refs_str, refs, yca_id, repo, 0); > if (err) > goto done; > } > blob - 41cee9ee65ad11031857077637457e6954320463 > blob + 7bdd9c2e0c5b082a18dcf878227aa026335384cf > --- regress/cmdline/log.sh > +++ regress/cmdline/log.sh > @@ -311,14 +311,18 @@ test_log_oneline() { > (cd $testroot/wt && got commit -m "test oneline > no" > /dev/null) > local commit_id1=`git_show_head $testroot/repo` > + local author_time1=`git_show_author_time $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` > + local author_time2=`git_show_author_time $testroot/repo` > > - printf "%.7s test oneline\n" $commit_id2 > $testroot/stdout.expected > - printf "%.7s test oneline\n" $commit_id1 >> $testroot/stdout.expected > + d=`date -u -r $author_time1 +"%G-%m-%d"` > + printf "$d %-7s test oneline\n" master > $testroot/stdout.expected > + d=`date -u -r $author_time2 +"%G-%m-%d"` > + printf "$d %.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