Hi Mark,

thanks for the review! Indeed, the part I doubted the most moving out
print_tag was the bad_sigs bit. I believe your suggestion still isn't
100% compatible with the previous version: originally, err could be
set to something on bad_sigs case (last iter + NULL commit). I decided
to instead check for GOT_ERR_BAD_TAG_SIGNATURE in order to break.

I adopted all your suggestion. The strcspn one is particulary nice.

As for dealing with a potentially NULL sig_msg, I decided to print "bad
signature" in that case.

I also switched to -S signer and -s for short. I adopted the object type
label, but went with 12-characters hash prefix and decided against
adding refs/tags/. If there is a function to get a short and unambiguous
hash prefix, I can use that instead.


-----------------------------------------------
commit c12c0e4ef0cb985af7d3daef54a96ebb42966e27
from: Lucas Gabriel Vuotto <lucas@sexy.is>
date: Mon Dec 23 17:00:22 2024 UTC
 
 got tag: change -s signer to -S signer
 
 We'll introduce a short output for tag listing and we'll use -s for
 consistency with got log. It also keeps it slightly more consistent with
 -V for verification.
 
 M  got/got.1
 M  got/got.c
 M  regress/cmdline/tag.sh

diff 589b5832ea0e12318af1af49fb76e2887629e2b6 c12c0e4ef0cb985af7d3daef54a96ebb42966e27
commit - 589b5832ea0e12318af1af49fb76e2887629e2b6
commit + c12c0e4ef0cb985af7d3daef54a96ebb42966e27
blob - e653af513ca5618a8e7dcf9459524bbb2f483e8d
blob + 6c0da5ae7b8d571bd4eb6e9bf5204d8a615e9d3f
--- got/got.1
+++ got/got.1
@@ -1729,7 +1729,7 @@ option to be used as well.
 .Op Fl c Ar commit
 .Op Fl m Ar message
 .Op Fl r Ar repository-path
-.Op Fl s Ar signer-id
+.Op Fl S Ar signer-id
 .Ar name
 .Xc
 Manage tags in a repository.
@@ -1821,7 +1821,7 @@ working directory.
 If this directory is a
 .Nm
 work tree, use the repository path associated with this work tree.
-.It Fl s Ar signer-id
+.It Fl S Ar signer-id
 While creating a new tag, sign this tag with the identity given in
 .Ar signer-id .
 .Pp
blob - 36655f1f322184d063373730e92b7dbf08b373a4
blob + b6dd736cec67d368d8bf2f51323d4bc76be2cb03
--- got/got.c
+++ got/got.c
@@ -7474,7 +7474,7 @@ __dead static void
 usage_tag(void)
 {
 	fprintf(stderr, "usage: %s tag [-lVv] [-c commit] [-m message] "
-	    "[-r repository-path] [-s signer-id] name\n", getprogname());
+	    "[-r repository-path] [-S signer-id] name\n", getprogname());
 	exit(1);
 }
 
@@ -7904,7 +7904,7 @@ cmd_tag(int argc, char *argv[])
 		err(1, "pledge");
 #endif
 
-	while ((ch = getopt(argc, argv, "c:lm:r:s:Vv")) != -1) {
+	while ((ch = getopt(argc, argv, "c:lm:r:S:Vv")) != -1) {
 		switch (ch) {
 		case 'c':
 			commit_id_arg = optarg;
@@ -7924,7 +7924,7 @@ cmd_tag(int argc, char *argv[])
 			}
 			got_path_strip_trailing_slashes(repo_path);
 			break;
-		case 's':
+		case 'S':
 			signer_id = optarg;
 			break;
 		case 'V':
blob - 005a6304020f8cb434ecb6fb450d1b44d9bcc66e
blob + cbf6d6c4be7cf8b0f5fafcaf3e258266d9f5cb7c
--- regress/cmdline/tag.sh
+++ regress/cmdline/tag.sh
@@ -295,7 +295,7 @@ test_tag_create_ssh_signed() {
 		$testroot/repo/.git/got.conf
 
 	# Create a signed tag based on repository's HEAD reference
-	got tag -s $testroot/id_ed25519 -m 'test' -r $testroot/repo -c HEAD \
+	got tag -S $testroot/id_ed25519 -m 'test' -r $testroot/repo -c HEAD \
 		$tag > $testroot/stdout
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -390,7 +390,7 @@ test_tag_create_ssh_signed() {
 	fi
 
 	# Create another signed tag with a SHA1 commit ID
-	got tag -s $testroot/id_ed25519 -m 'test' -r $testroot/repo \
+	got tag -S $testroot/id_ed25519 -m 'test' -r $testroot/repo \
 		-c $commit_id $tag2 > $testroot/stdout
 
 	# Create another signed tag with key defined in got.conf(5)
@@ -435,7 +435,7 @@ test_tag_create_ssh_signed_missing_key() {
 	local tag=1.0.0
 
 	# Fail to create a signed tag due to a missing SSH key
-	got tag -s $testroot/bogus -m 'test' -r $testroot/repo \
+	got tag -S $testroot/bogus -m 'test' -r $testroot/repo \
 		-c HEAD	$tag > $testroot/stdout 2> $testroot/stderr
 	ret=$?
 	if [ $ret -eq 0 ]; then

-----------------------------------------------
commit 597e18771385dec048278eb4b9bc25edc803ee69
from: Lucas Gabriel Vuotto <lucas@sexy.is>
date: Mon Dec 23 17:00:35 2024 UTC
 
 got tag: move tag printing into its own function
 
 in prepartion for a oneline output mode
 
 M  got/got.c

diff c12c0e4ef0cb985af7d3daef54a96ebb42966e27 597e18771385dec048278eb4b9bc25edc803ee69
commit - c12c0e4ef0cb985af7d3daef54a96ebb42966e27
commit + 597e18771385dec048278eb4b9bc25edc803ee69
blob - b6dd736cec67d368d8bf2f51323d4bc76be2cb03
blob + 7616f8b1155aa3346751e64173119bbcb0f7ba4b
--- got/got.c
+++ got/got.c
@@ -7554,6 +7554,78 @@ get_tag_refname(char **refname, const char *tag_name)
 }
 
 static const struct got_error *
+print_tag(struct got_tag_object *tag, struct got_commit_object *commit,
+    const char *refname, const char *refstr, const char *tagger,
+    time_t tagger_time, const char *id_str, const char *ssh_sig,
+    const char *allowed_signers, const char *revoked_signers, int verbosity,
+    int *bad_sigs)
+{
+	static const struct got_error *err = NULL;
+	char datebuf[26];
+	char *sig_msg = NULL, *tagmsg0 = NULL, *tagmsg, *line, *datestr;
+
+	printf("%stag %s %s\n", GOT_COMMIT_SEP_STR, refname, refstr);
+	printf("from: %s\n", tagger);
+	datestr = get_datestr(&tagger_time, datebuf);
+	if (datestr)
+		printf("date: %s UTC\n", datestr);
+	if (commit)
+		printf("object: %s %s\n", GOT_OBJ_LABEL_COMMIT, id_str);
+	else {
+		switch (got_object_tag_get_object_type(tag)) {
+		case GOT_OBJ_TYPE_BLOB:
+			printf("object: %s %s\n", GOT_OBJ_LABEL_BLOB, id_str);
+			break;
+		case GOT_OBJ_TYPE_TREE:
+			printf("object: %s %s\n", GOT_OBJ_LABEL_TREE, id_str);
+			break;
+		case GOT_OBJ_TYPE_COMMIT:
+			printf("object: %s %s\n", GOT_OBJ_LABEL_COMMIT, id_str);
+			break;
+		case GOT_OBJ_TYPE_TAG:
+			printf("object: %s %s\n", GOT_OBJ_LABEL_TAG, id_str);
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (ssh_sig) {
+		err = got_sigs_verify_tag_ssh(&sig_msg, tag, ssh_sig,
+		    allowed_signers, revoked_signers, verbosity);
+		if (err && err->code == GOT_ERR_BAD_TAG_SIGNATURE)
+			*bad_sigs = 1;
+		else if (err)
+			goto done;
+		if (sig_msg)
+			printf("signature: %s", sig_msg);
+		else
+			printf("bad signature\n");
+		free(sig_msg);
+	}
+
+	if (commit) {
+		err = got_object_commit_get_logmsg(&tagmsg0, commit);
+		if (err)
+			goto done;
+	} else {
+		tagmsg0 = strdup(got_object_tag_get_message(tag));
+		if (tagmsg0 == NULL) {
+			err = got_error_from_errno("strdup");
+			goto done;
+		}
+	}
+
+	tagmsg = tagmsg0;
+	while ((line = strsep(&tagmsg, "\n")) != NULL)
+		printf(" %s\n", line);
+
+ done:
+	free(tagmsg0);
+	return err;
+}
+
+static const struct got_error *
 list_tags(struct got_repository *repo, const char *tag_name, int verify_tags,
     const char *allowed_signers, const char *revoked_signers, int verbosity)
 {
@@ -7583,10 +7655,8 @@ list_tags(struct got_repository *repo, const char *tag
 
 	TAILQ_FOREACH(re, &refs, entry) {
 		const char *refname;
-		char *refstr, *tagmsg0, *tagmsg, *line, *id_str, *datestr;
-		char datebuf[26];
+		char *refstr, *id_str;
 		const char *tagger, *ssh_sig = NULL;
-		char *sig_msg = NULL;
 		time_t tagger_time;
 		struct got_object_id *id;
 		struct got_tag_object *tag;
@@ -7648,71 +7718,19 @@ list_tags(struct got_repository *repo, const char *tag
 			}
 		}
 
-		printf("%stag %s %s\n", GOT_COMMIT_SEP_STR, refname, refstr);
-		free(refstr);
-		printf("from: %s\n", tagger);
-		datestr = get_datestr(&tagger_time, datebuf);
-		if (datestr)
-			printf("date: %s UTC\n", datestr);
+		err = print_tag(tag, commit, refname, refstr, tagger,
+		    tagger_time, id_str, ssh_sig, allowed_signers,
+		    revoked_signers, verbosity, &bad_sigs);
+
 		if (commit)
-			printf("object: %s %s\n", GOT_OBJ_LABEL_COMMIT, id_str);
-		else {
-			switch (got_object_tag_get_object_type(tag)) {
-			case GOT_OBJ_TYPE_BLOB:
-				printf("object: %s %s\n", GOT_OBJ_LABEL_BLOB,
-				    id_str);
-				break;
-			case GOT_OBJ_TYPE_TREE:
-				printf("object: %s %s\n", GOT_OBJ_LABEL_TREE,
-				    id_str);
-				break;
-			case GOT_OBJ_TYPE_COMMIT:
-				printf("object: %s %s\n", GOT_OBJ_LABEL_COMMIT,
-				    id_str);
-				break;
-			case GOT_OBJ_TYPE_TAG:
-				printf("object: %s %s\n", GOT_OBJ_LABEL_TAG,
-				    id_str);
-				break;
-			default:
-				break;
-			}
-		}
-		free(id_str);
-
-		if (ssh_sig) {
-			err = got_sigs_verify_tag_ssh(&sig_msg, tag, ssh_sig,
-				allowed_signers, revoked_signers, verbosity);
-			if (err && err->code == GOT_ERR_BAD_TAG_SIGNATURE)
-				bad_sigs = 1;
-			else if (err)
-				break;
-			printf("signature: %s", sig_msg);
-			free(sig_msg);
-			sig_msg = NULL;
-		}
-
-		if (commit) {
-			err = got_object_commit_get_logmsg(&tagmsg0, commit);
-			if (err)
-				break;
 			got_object_commit_close(commit);
-		} else {
-			tagmsg0 = strdup(got_object_tag_get_message(tag));
+		if (tag)
 			got_object_tag_close(tag);
-			if (tagmsg0 == NULL) {
-				err = got_error_from_errno("strdup");
-				break;
-			}
-		}
+		free(id_str);
+		free(refstr);
 
-		tagmsg = tagmsg0;
-		do {
-			line = strsep(&tagmsg, "\n");
-			if (line)
-				printf(" %s\n", line);
-		} while (line);
-		free(tagmsg0);
+		if (err && err->code != GOT_ERR_BAD_TAG_SIGNATURE)
+			break;
 	}
 done:
 	got_ref_list_free(&refs);

-----------------------------------------------
commit fe61c0726471e67e9b3fb62117565ae3e7374b99 (tag-short)
from: Lucas Gabriel Vuotto <lucas@sexy.is>
date: Mon Dec 23 17:00:35 2024 UTC
 
 got tag: provide one-line output mode
 
 M  got/got.1
 M  got/got.c
 M  regress/cmdline/tag.sh

diff 597e18771385dec048278eb4b9bc25edc803ee69 fe61c0726471e67e9b3fb62117565ae3e7374b99
commit - 597e18771385dec048278eb4b9bc25edc803ee69
commit + fe61c0726471e67e9b3fb62117565ae3e7374b99
blob - 6c0da5ae7b8d571bd4eb6e9bf5204d8a615e9d3f
blob + 96794d6aa787f704f409ac79ea9ef7d1ef02caee
--- got/got.1
+++ got/got.1
@@ -1725,7 +1725,7 @@ option to be used as well.
 .El
 .It Xo
 .Cm tag
-.Op Fl lVv
+.Op Fl lsVv
 .Op Fl c Ar commit
 .Op Fl m Ar message
 .Op Fl r Ar repository-path
@@ -1839,6 +1839,12 @@ command, using the signature namespace
 .Dq git
 for compatibility with
 .Xr git 1 .
+.It Fl s
+Display a short one-line summary of each tag, instead of the default
+history format.
+Can only be used with the
+.Fl l
+option.
 .It Fl V
 Verify tag object signatures.
 If a
blob - 7616f8b1155aa3346751e64173119bbcb0f7ba4b
blob + faeb2a45b6551a6fd8e88548770ef14de3684766
--- got/got.c
+++ got/got.c
@@ -7473,7 +7473,7 @@ done:
 __dead static void
 usage_tag(void)
 {
-	fprintf(stderr, "usage: %s tag [-lVv] [-c commit] [-m message] "
+	fprintf(stderr, "usage: %s tag [-lsVv] [-c commit] [-m message] "
 	    "[-r repository-path] [-S signer-id] name\n", getprogname());
 	exit(1);
 }
@@ -7554,6 +7554,70 @@ get_tag_refname(char **refname, const char *tag_name)
 }
 
 static const struct got_error *
+print_tag_oneline(struct got_tag_object *tag, struct got_commit_object *commit,
+    const char *refname, const char *refstr, time_t tagger_time,
+    const char *id_str)
+{
+	static const struct got_error *err = NULL;
+	const char *label = NULL;
+	struct tm tm;
+	char *tagmsg0 = NULL, *tagmsg;
+	char datebuf[11];
+
+	if (gmtime_r(&tagger_time, &tm) == NULL)
+		return got_error_from_errno("gmtime_r");
+	if (strftime(datebuf, sizeof(datebuf), "%F", &tm) == 0)
+		return got_error(GOT_ERR_NO_SPACE);
+
+	if (commit) {
+		err = got_object_commit_get_logmsg(&tagmsg0, commit);
+		if (err)
+			return err;
+	} else {
+		tagmsg0 = strdup(got_object_tag_get_message(tag));
+		if (tagmsg0 == NULL)
+			return got_error_from_errno("strdup");
+	}
+
+	tagmsg = tagmsg0;
+	while (isspace((unsigned char)*tagmsg))
+		tagmsg++;
+	tagmsg[strcspn(tagmsg, "\n")] = '\0';
+
+	if (commit)
+		label = GOT_OBJ_LABEL_COMMIT;
+	else {
+		switch (got_object_tag_get_object_type(tag)) {
+		case GOT_OBJ_TYPE_BLOB:
+			label = GOT_OBJ_LABEL_BLOB;
+			break;
+		case GOT_OBJ_TYPE_TREE:
+			label = GOT_OBJ_LABEL_TREE;
+			break;
+		case GOT_OBJ_TYPE_COMMIT:
+			label = GOT_OBJ_LABEL_COMMIT;
+			break;
+		case GOT_OBJ_TYPE_TAG:
+			label = GOT_OBJ_LABEL_TAG;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (label)
+		printf("%s %s:%.12s %s: %s\n", datebuf, label, id_str, refname,
+		    tagmsg);
+	else
+		printf("%s tag:%.12s %s: %s\n", datebuf, refstr, refname,
+		    tagmsg);
+
+	free(tagmsg0);
+
+	return err;
+}
+
+static const struct got_error *
 print_tag(struct got_tag_object *tag, struct got_commit_object *commit,
     const char *refname, const char *refstr, const char *tagger,
     time_t tagger_time, const char *id_str, const char *ssh_sig,
@@ -7627,7 +7691,8 @@ print_tag(struct got_tag_object *tag, struct got_commi
 
 static const struct got_error *
 list_tags(struct got_repository *repo, const char *tag_name, int verify_tags,
-    const char *allowed_signers, const char *revoked_signers, int verbosity)
+    const char *allowed_signers, const char *revoked_signers, int verbosity,
+    int oneline)
 {
 	static const struct got_error *err = NULL;
 	struct got_reflist_head refs;
@@ -7718,9 +7783,13 @@ list_tags(struct got_repository *repo, const char *tag
 			}
 		}
 
-		err = print_tag(tag, commit, refname, refstr, tagger,
-		    tagger_time, id_str, ssh_sig, allowed_signers,
-		    revoked_signers, verbosity, &bad_sigs);
+		if (oneline)
+			err = print_tag_oneline(tag, commit, refname, refstr,
+			    tagger_time, id_str);
+		else
+			err = print_tag(tag, commit, refname, refstr, tagger,
+			    tagger_time, id_str, ssh_sig, allowed_signers,
+			    revoked_signers, verbosity, &bad_sigs);
 
 		if (commit)
 			got_object_commit_close(commit);
@@ -7913,7 +7982,7 @@ cmd_tag(int argc, char *argv[])
 	char *allowed_signers = NULL, *revoked_signers = NULL, *editor = NULL;
 	const char *signer_id = NULL;
 	const char *tag_name = NULL, *commit_id_arg = NULL, *tagmsg = NULL;
-	int ch, do_list = 0, verify_tags = 0, verbosity = 0;
+	int ch, do_list = 0, oneline = 0, verify_tags = 0, verbosity = 0;
 	int *pack_fds = NULL;
 
 #ifndef PROFILE
@@ -7922,7 +7991,7 @@ cmd_tag(int argc, char *argv[])
 		err(1, "pledge");
 #endif
 
-	while ((ch = getopt(argc, argv, "c:lm:r:S:Vv")) != -1) {
+	while ((ch = getopt(argc, argv, "c:lm:r:S:sVv")) != -1) {
 		switch (ch) {
 		case 'c':
 			commit_id_arg = optarg;
@@ -7945,6 +8014,9 @@ cmd_tag(int argc, char *argv[])
 		case 'S':
 			signer_id = optarg;
 			break;
+		case 's':
+			oneline = 1;
+			break;
 		case 'V':
 			verify_tags = 1;
 			break;
@@ -7963,7 +8035,9 @@ cmd_tag(int argc, char *argv[])
 	argc -= optind;
 	argv += optind;
 
-	if (do_list || verify_tags) {
+	if (oneline && !do_list)
+		errx(1, "-S option con only be used when listing tags");
+	else if (do_list || verify_tags) {
 		if (commit_id_arg != NULL)
 			errx(1,
 			    "-c option can only be used when creating a tag");
@@ -8053,7 +8127,7 @@ cmd_tag(int argc, char *argv[])
 		if (error)
 			goto done;
 		error = list_tags(repo, tag_name, verify_tags, allowed_signers,
-		    revoked_signers, verbosity);
+		    revoked_signers, verbosity, oneline);
 	} else {
 		error = get_gitconfig_path(&gitconfig_path);
 		if (error)
blob - cbf6d6c4be7cf8b0f5fafcaf3e258266d9f5cb7c
blob + 2ace9e32e901960e9507e544db469d4be5139629
--- regress/cmdline/tag.sh
+++ regress/cmdline/tag.sh
@@ -223,6 +223,60 @@ test_tag_list() {
 	test_done "$testroot" "$ret"
 }
 
+test_tag_list_oneline() {
+	local testroot=`test_init tag_list`
+	local commit_id=`git_show_head $testroot/repo`
+	local tag=1.0.0
+	local tag2=2.0.0
+
+	local head_ref=`got ref -r $testroot/repo -l | sed -n 's/^HEAD: //p'`
+	local tag_commit_id=`got ref -r $testroot/repo -l "$head_ref" |
+	    sed -n "s:^$head_ref\: ::p"`
+
+	# create tag with Git
+	git -C $testroot/repo tag -a -m 'test' $tag
+	# create tag with Got
+	(cd $testroot/repo && got tag -m 'test' $tag2 > /dev/null)
+
+	local tagger_time=`git_show_tagger_time $testroot/repo $tag`
+	d1=`date -u -r $tagger_time +"%F"`
+	local tagger_time2=`git_show_tagger_time $testroot/repo $tag2`
+	d2=`date -u -r $tagger_time2 +"%F"`
+
+	got tag -r $testroot/repo -ls > $testroot/stdout
+
+	echo "$d2 commit:$(trim_obj_id 12 "$tag_commit_id") $tag2: test" > $testroot/stdout.expected
+	echo "$d1 commit:$(trim_obj_id 12 "$tag_commit_id") $tag: test" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got tag -r $testroot/repo -ls $tag > $testroot/stdout
+
+	echo "$d1 commit:$(trim_obj_id 12 "$tag_commit_id") $tag: test" > $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got tag -r $testroot/repo -ls $tag2 > $testroot/stdout
+
+	echo "$d2 commit:$(trim_obj_id 12 "$tag_commit_id") $tag2: test" > $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
 test_tag_list_lightweight() {
 	local testroot=`test_init tag_list_lightweight`
 	local commit_id=`git_show_head $testroot/repo`
@@ -573,6 +627,7 @@ test_parseargs "$@"
 run_test test_tag_create
 run_test test_tag_list
 run_test test_tag_list_lightweight
+run_test test_tag_list_oneline
 run_test test_tag_create_ssh_signed
 run_test test_tag_create_ssh_signed_missing_key
 run_test test_tag_commit_keywords