On 22-07-01 12:32pm, Stefan Sperling wrote:
> On Fri, Jul 01, 2022 at 03:15:31PM +1000, Mark Jamsek wrote:
> > On 22-07-01 12:15am, Stefan Sperling wrote:
> > > Make the diff algorithm used by 'tog diff' and 'tog blame' configurable.
> > > This allows people (like me) who prefer slow and tidy Patience diffs over
> > > quick and ugly Myers diffs to tell tog about this preference.
> > > 
> > > As a bonus, the diff algorithm can be toggled at run-time, in both the
> > > diff view and the blame view, by pressing the A key.
> > > 
> > > ok?
> > 
> > Nice! Works as described.
> > 
> > ok with or without tiny nit below
> 
> Thanks!
> 
> The previous diff had a problem. When both a blame and a diff view were
> displayed, and the diff algorithm was switched in one of the two views,
> only the view which was processing input got updated. The other view was
> now out of sync with the current diff algorithm setting.
> 
> The new patch below does not have this problem. If the diff algorithm is
> changed in any view, all relevant views will be regenerated.

Reads nicer too. Makes sense doing this in view_input() now.

I think you left a couple fprintf()s in reset_{blame,diff}_view();
otherwise ok.

> diff 4b752015b5208a96c2d1b6c1c6b8589884b8b2b6 026bef347ee0d2a3a411a47420b9aec1a00aec3e
> commit - 4b752015b5208a96c2d1b6c1c6b8589884b8b2b6
> commit + 026bef347ee0d2a3a411a47420b9aec1a00aec3e
> blob - 728ba5ee9e20edbde20ed7f0c2ff1cbabad89c0b
> blob + ed0412fd13daaa1ea18139730716db483ba49274
> --- tog/tog.1
> +++ tog/tog.1
> @@ -295,6 +295,12 @@ Find the Nth previous line which matches the current s
>  (default: 1).
>  .It Cm w
>  Toggle display of whitespace-only changes.
> +.It Cm A
> +Change the diff algorithm.
> +Supported diff algorithms are Myers and Patience. 
> +This is a global setting which also affects the
> +.Cm blame
> +view.
>  .El
>  .Pp
>  The options for
> @@ -379,6 +385,12 @@ Find the Nth next line which matches the current searc
>  .It Cm N
>  Find the Nth previous line which matches the current search pattern
>  (default: 1).
> +.It Cm A
> +Change the diff algorithm.
> +Supported diff algorithms are Myers and Patience. 
> +This is a global setting which also affects the
> +.Cm diff
> +view.
>  .El
>  .Pp
>  The options for
> @@ -560,7 +572,15 @@ work tree, use the repository path associated with thi
>  .El
>  .El
>  .Sh ENVIRONMENT
> -.Bl -tag -width TOG_COLORS
> +.Bl -tag -width TOG_DIFF_ALGORITHM
> +.It Ev TOG_DIFF_ALGORITHM
> +Determines the default diff algorithm used by
> +.Nm .
> +Valid values are
> +.Dq patience
> +and
> +.Dq myers .
> +If unset, the Myers diff algorithm will be used by default.
>  .It Ev TOG_COLORS
>  .Nm
>  shows colorized output if this variable is set to a non-empty value.
> blob - 800384dacf0faed4893832dd2760af45e28cb8c7
> blob + 56481e67ad1e36e598bf01561c6b3ec76fea52c2
> --- tog/tog.c
> +++ tog/tog.c
> @@ -136,6 +136,7 @@ STAILQ_HEAD(tog_colors, tog_color);
>  
>  static struct got_reflist_head tog_refs = TAILQ_HEAD_INITIALIZER(tog_refs);
>  static struct got_reflist_object_id_map *tog_refs_idmap;
> +static enum got_diff_algorithm tog_diff_algo = GOT_DIFF_ALGORITHM_MYERS;
>  
>  static const struct got_error *
>  tog_ref_cmp_by_name(void *arg, int *cmp, struct got_reference *re1,
> @@ -321,7 +322,6 @@ struct tog_diff_view_state {
>  	int first_displayed_line;
>  	int last_displayed_line;
>  	int eof;
> -	enum got_diff_algorithm diff_algo;
>  	int diff_context;
>  	int ignore_whitespace;
>  	int force_text_diff;
> @@ -544,6 +544,7 @@ struct tog_view {
>  	const struct got_error *(*show)(struct tog_view *);
>  	const struct got_error *(*input)(struct tog_view **,
>  	    struct tog_view *, int);
> +	const struct got_error *(*reset)(struct tog_view *);
>  	const struct got_error *(*close)(struct tog_view *);
>  
>  	const struct got_error *(*search_start)(struct tog_view *);
> @@ -567,6 +568,7 @@ static const struct got_error *open_diff_view(struct t
>  static const struct got_error *show_diff_view(struct tog_view *);
>  static const struct got_error *input_diff_view(struct tog_view **,
>      struct tog_view *, int);
> +static const struct got_error *reset_diff_view(struct tog_view *);
>  static const struct got_error* close_diff_view(struct tog_view *);
>  static const struct got_error *search_start_diff_view(struct tog_view *);
>  static const struct got_error *search_next_diff_view(struct tog_view *);
> @@ -586,6 +588,7 @@ static const struct got_error *open_blame_view(struct 
>  static const struct got_error *show_blame_view(struct tog_view *);
>  static const struct got_error *input_blame_view(struct tog_view **,
>      struct tog_view *, int);
> +static const struct got_error *reset_blame_view(struct tog_view *);
>  static const struct got_error *close_blame_view(struct tog_view *);
>  static const struct got_error *search_start_blame_view(struct tog_view *);
>  static const struct got_error *search_next_blame_view(struct tog_view *);
> @@ -651,7 +654,6 @@ tog_fatal_signal_received(void)
>  	    tog_sigint_received || tog_sigint_received);
>  }
>  
> -
>  static const struct got_error *
>  view_close(struct tog_view *view)
>  {
> @@ -1217,6 +1219,24 @@ view_input(struct tog_view **new, int *done, struct to
>  		} else
>  			err = view->input(new, view, ch);
>  		break;
> +	case 'A':
> +		if (tog_diff_algo == GOT_DIFF_ALGORITHM_MYERS)
> +			tog_diff_algo = GOT_DIFF_ALGORITHM_PATIENCE;
> +		else
> +			tog_diff_algo = GOT_DIFF_ALGORITHM_MYERS;
> +		TAILQ_FOREACH(v, views, entry) {
> +			if (v->reset) {
> +				err = v->reset(v);
> +				if (err)
> +					return err;
> +			}
> +			if (v->child && v->child->reset) {
> +				err = v->child->reset(v->child);
> +				if (err)
> +					return err;
> +			}
> +		}
> +		break;
>  	default:
>  		err = view->input(new, view, ch);
>  		break;
> @@ -3966,13 +3986,13 @@ create_diff(struct tog_diff_view_state *s)
>  	case GOT_OBJ_TYPE_BLOB:
>  		err = got_diff_objects_as_blobs(&s->line_offsets, &s->nlines,
>  		    s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2,
> -		    s->label1, s->label2, s->diff_algo, s->diff_context,
> +		    s->label1, s->label2, tog_diff_algo, s->diff_context,
>  		    s->ignore_whitespace, s->force_text_diff, s->repo, s->f);
>  		break;
>  	case GOT_OBJ_TYPE_TREE:
>  		err = got_diff_objects_as_trees(&s->line_offsets, &s->nlines,
>  		    s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2, NULL, "", "",
> -		    s->diff_algo, s->diff_context, s->ignore_whitespace,
> +		    tog_diff_algo, s->diff_context, s->ignore_whitespace,
>  		    s->force_text_diff, s->repo, s->f);
>  		break;
>  	case GOT_OBJ_TYPE_COMMIT: {
> @@ -4008,7 +4028,7 @@ create_diff(struct tog_diff_view_state *s)
>  
>  		err = got_diff_objects_as_commits(&s->line_offsets, &s->nlines,
>  		    s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2, NULL,
> -		    s->diff_algo, s->diff_context, s->ignore_whitespace,
> +		    tog_diff_algo, s->diff_context, s->ignore_whitespace,
>  		    s->force_text_diff, s->repo, s->f);
>  		break;
>  	}
> @@ -4219,7 +4239,6 @@ open_diff_view(struct tog_view *view, struct got_objec
>  
>  	s->first_displayed_line = 1;
>  	s->last_displayed_line = view->nlines;
> -	s->diff_algo = GOT_DIFF_ALGORITHM_MYERS;
>  	s->diff_context = diff_context;
>  	s->ignore_whitespace = ignore_whitespace;
>  	s->force_text_diff = force_text_diff;
> @@ -4273,6 +4292,7 @@ open_diff_view(struct tog_view *view, struct got_objec
>  
>  	view->show = show_diff_view;
>  	view->input = input_diff_view;
> +	view->reset = reset_diff_view;
>  	view->close = close_diff_view;
>  	view->search_start = search_start_diff_view;
>  	view->search_next = search_next_diff_view;
> @@ -4343,6 +4363,22 @@ set_selected_commit(struct tog_diff_view_state *s,
>  }
>  
>  static const struct got_error *
> +reset_diff_view(struct tog_view *view)
> +{
> +	struct tog_diff_view_state *s = &view->state.diff;
> +
> +	fprintf(stderr, "%s\n", __func__);

here    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> +	view->count = 0;
> +	wclear(view->window);
> +	s->first_displayed_line = 1;
> +	s->last_displayed_line = view->nlines;
> +	s->matched_line = 0;
> +	diff_view_indicate_progress(view);
> +	return create_diff(s);
> +}
> +
> +static const struct got_error *
>  input_diff_view(struct tog_view **new_view, struct tog_view *view, int ch)
>  {
>  	const struct got_error *err = NULL;
> @@ -4381,13 +4417,7 @@ input_diff_view(struct tog_view **new_view, struct tog
>  			s->force_text_diff = !s->force_text_diff;
>  		if (ch == 'w')
>  			s->ignore_whitespace = !s->ignore_whitespace;
> -		wclear(view->window);
> -		s->first_displayed_line = 1;
> -		s->last_displayed_line = view->nlines;
> -		s->matched_line = 0;
> -		diff_view_indicate_progress(view);
> -		err = create_diff(s);
> -		view->count = 0;
> +		err = reset_diff_view(view);
>  		break;
>  	case 'g':
>  	case KEY_HOME:
> @@ -4950,7 +4980,7 @@ blame_thread(void *arg)
>  		goto done;
>  
>  	err = got_blame(ta->path, a->commit_id, ta->repo,
> -	    GOT_DIFF_ALGORITHM_MYERS, blame_cb, ta->cb_args,
> +	    tog_diff_algo, blame_cb, ta->cb_args,
>  	    ta->cancel_cb, ta->cancel_arg, fd1, fd2, f1, f2);
>  	if (err && err->code == GOT_ERR_CANCELLED)
>  		err = NULL;
> @@ -5225,6 +5255,7 @@ open_blame_view(struct tog_view *view, char *path,
>  
>  	view->show = show_blame_view;
>  	view->input = input_blame_view;
> +	view->reset = reset_blame_view;
>  	view->close = close_blame_view;
>  	view->search_start = search_start_blame_view;
>  	view->search_next = search_next_blame_view;
> @@ -5641,6 +5672,22 @@ input_blame_view(struct tog_view **new_view, struct to
>  }
>  
>  static const struct got_error *
> +reset_blame_view(struct tog_view *view)
> +{
> +	const struct got_error *err;
> +	struct tog_blame_view_state *s = &view->state.blame;
> +
> +	fprintf(stderr, "%s\n", __func__);

here    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> +	view->count = 0;
> +	s->done = 1;
> +	err = stop_blame(&s->blame);
> +	s->done = 0;
> +	if (err)
> +		return err;
> +	return run_blame(view);
> +}
> +
> +static const struct got_error *
>  cmd_blame(int argc, char *argv[])
>  {
>  	const struct got_error *error;
> @@ -7839,6 +7886,7 @@ main(int argc, char *argv[])
>  	    { "version", no_argument, NULL, 'V' },
>  	    { NULL, 0, NULL, 0}
>  	};
> +	char *diff_algo_str = NULL;
>  
>  	setlocale(LC_CTYPE, "");
>  
> @@ -7892,6 +7940,14 @@ main(int argc, char *argv[])
>  		}
>  	}
>  
> +	diff_algo_str = getenv("TOG_DIFF_ALGORITHM");
> +	if (diff_algo_str) {
> +		if (strcasecmp(diff_algo_str, "patience") == 0)
> +			tog_diff_algo = GOT_DIFF_ALGORITHM_PATIENCE;
> +		if (strcasecmp(diff_algo_str, "myers") == 0)
> +			tog_diff_algo = GOT_DIFF_ALGORITHM_MYERS;
> +	}
> +
>  	if (cmd == NULL) {
>  		if (argc != 1)
>  			usage(0, 1);
> 

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