On 22-07-19 11:01am, Omar Polo wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
> > On Tue, Jul 19, 2022 at 10:34:16AM +0200, Omar Polo wrote:
> > > it may be marginally useful to have this on the blame view too but
> > > otherwise no, there aren't reasons not to keep this in the log state
> > > struct.
> > 
> > The blame view doesn't show author info at present, it only lists
> > commit IDs. If we wanted to add author/date columns later then a
> > toggle to switch between author/committer might be useful indeed.
> > 
> > > i've just remembered that in the blame view the 'C' key is already
> > > taken, should we use a different key for this toggle from the beginning?
> > 
> > I guess we could use '@' for switching between author/committer.
> > This would probably be mnemonic enough.
> > 
> > Or we could change 'A' (diff algorithm) to something else (e.g. '#' or
> > '@'), and then use the 'A' key for author/committer. This would match
> > your proposed 'got commit -A' option, though we don't need to require
> > this to match. Perhaps this option could be named -@ instead of -A if
> > we go with the first idea and want got and tog to be consistent.
> > 
> > Not sure. I do not see a clear best way to do it.
> > What would you prefer?
> 
> sold for the '@' key.  I wanted to use A but since it was taken i went
> with C; @ is more mnemonic and unused, so we may also use it in the
> future for an hypothetic blame view with author info.
> 
> keeping the got commit flag in sync with the tog is nice, but -@ is a
> bit ugly imho so i guess we can live with the (small) inconsintency of
> having got ci -A and tog @ :)

I like the '@' key too :)

Tiny nit inline, but ok either way.

> diff refs/heads/main refs/heads/togauthor
> commit - d2587c5f95c6edb51ccc8d4abfac838b58f3a463
> commit + e5c740fadc90347bcb1645c3c0ce4ea12606ba35
> blob - 4ec26b8962b911a5c478faf7aebc3d0686931d6e
> blob + 765c5ecd53d258c8af195523d4fb1fea0c8f41d3
> --- tog/tog.1
> +++ tog/tog.1
> @@ -214,6 +214,8 @@ view listing all references in the repository.
>  This can then be used to open a new
>  .Cm log
>  view for arbitrary branches and tags.
> +.It Cm @
> +Toggle between showing the author and the committer name.
>  .El
>  .Pp
>  The options for
> blob - 97ac0690a9232815bb10bc4a237246e2ec53e069
> blob + 7ce5f28a4f4a2de29a94c09194188fc8703f7080
> --- tog/tog.c
> +++ tog/tog.c
> @@ -376,6 +376,7 @@ struct tog_log_view_state {
>  	struct commit_queue_entry *matched_entry;
>  	struct commit_queue_entry *search_entry;
>  	struct tog_colors colors;
> +	int use_committer;
>  };
>  
>  #define TOG_COLOR_DIFF_MINUS		1
> @@ -1962,7 +1963,10 @@ draw_commit(struct tog_view *view, struct got_commit_o
>  			goto done;
>  	}
>  
> -	author = strdup(got_object_commit_get_author(commit));
> +	if (view->state.log.use_committer)

We have s assigned view->state.log in scope here.

> +		author = strdup(got_object_commit_get_committer(commit));
> +	else
> +		author = strdup(got_object_commit_get_author(commit));
>  	if (author == NULL) {
>  		err = got_error_from_errno("strdup");
>  		goto done;
> @@ -2272,12 +2276,16 @@ draw_commits(struct tog_view *view)
>  	ncommits = 0;
>  	view->maxx = 0;
>  	while (entry) {
> +		struct got_commit_object *c = entry->commit;
>  		char *author, *eol, *msg, *msg0;
>  		wchar_t *wauthor, *wmsg;
>  		int width;
>  		if (ncommits >= limit - 1)
>  			break;
> -		author = strdup(got_object_commit_get_author(entry->commit));
> +		if (view->state.log.use_committer)

And here too.

> +			author = strdup(got_object_commit_get_committer(c));
> +		else
> +			author = strdup(got_object_commit_get_author(c));
>  		if (author == NULL) {
>  			err = got_error_from_errno("strdup");
>  			goto done;
> @@ -2288,7 +2296,7 @@ draw_commits(struct tog_view *view)
>  			author_cols = width;
>  		free(wauthor);
>  		free(author);
> -		err = got_object_commit_get_logmsg(&msg0, entry->commit);
> +		err = got_object_commit_get_logmsg(&msg0, c);
>  		if (err)
>  			goto done;
>  		msg = msg0;
> @@ -3258,6 +3266,9 @@ input_log_view(struct tog_view **new_view, struct tog_
>  	case CTRL('n'):
>  		err = log_move_cursor_down(view, 0);
>  		break;
> +	case '@':
> +		view->state.log.use_committer = !view->state.log.use_committer;
> +		break;
>  	case 'G':
>  	case KEY_END: {
>  		/* We don't know yet how many commits, so we're forced to
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68