Subject: Re: [PATCH v2 0/2] improve interactive-patch

From: Rubén Justo <[email-masked]>
Date: Sat, 30 Mar 2024 14:49:16 +0100

On Fri, Mar 29, 2024 at 08:48:18PM +0100, Dragan Simic wrote:
> On 2024-03-29 20:26, Rubén Justo wrote:
> > On Tue, Mar 26, 2024 at 12:13:46PM -0700, Junio C Hamano wrote:
> > > > On Tue, Mar 26, 2024 at 08:31:41AM -0700, Junio C Hamano wrote:
> > > >
> > > >> 'r'edisplay may work well (and I wonder "r | less" or
> > > >> piping the hunk display to anything in general would be a useful
> > > >> future enhancement).
> > > >
> > > 
> > > It would be more like tweaking fputs() of a strbuf that
> > > was filled by render_hunk() to instead spawn a pager and feed the
> > > same strbuf to it, or something.  IOW, we already have the payload
> > > to show.  We just want a pager involved in its showing so that users
> > > with a huge hunk that does not fit on a page can use "less" on it.
> > 
> > I do not plan to address this in this series, but while the topic is
> > warm;  Perhaps?:
> 
> As a note, I find that having chunks displayed through the pager
> would be really nice.

I'm glad you find it useful.

I'm not going to include it in this series.  However, I've been using it
these days, and it's been fulfilling my needs.

I'm not sure of the implementation, though.

For a start, fputs_to_pager_or_stdout may be a better name, but ugly.

And "pager.h" may not be the right place to have it.

Perhaps something based on a strbuf is a better and more usable approach.

To name a few ...

> 
> > --- >8 ---
> > diff --git a/add-patch.c b/add-patch.c
> > index 778f168298..cb74fe84f5 100644
> > --- a/add-patch.c
> > +++ b/add-patch.c
> > @@ -5,6 +5,7 @@
> >  #include "environment.h"
> >  #include "gettext.h"
> >  #include "object-name.h"
> > +#include "pager.h"
> >  #include "read-cache-ll.h"
> >  #include "repository.h"
> >  #include "strbuf.h"
> > @@ -1450,7 +1451,7 @@ static int patch_update_file(struct add_p_state
> > *s,
> >  		if (file_diff->hunk_nr) {
> >  			if (rendered_hunk_index != hunk_index) {
> >  				render_hunk(s, hunk, 0, colored, &s->buf);
> > -				fputs(s->buf.buf, stdout);
> > +				fputs_to_pager(s->buf.buf);
> >  				rendered_hunk_index = hunk_index;
> >  			}
> > 
> > diff --git a/pager.c b/pager.c
> > index b8822a9381..f00fc87a67 100644
> > --- a/pager.c
> > +++ b/pager.c
> > @@ -264,3 +264,30 @@ int check_pager_config(const char *cmd)
> >  		pager_program = data.value;
> >  	return data.want;
> >  }
> > +
> > +void fputs_to_pager(const char* s)
> > +{
> > +	struct child_process process;
> > +	FILE* pager_stdin;
> > +	const char *pager = git_pager(isatty(1));
> > +
> > +	if (!pager) {
> > +		fputs(s, stdout);
> > +		return;
> > +	}
> > +
> > +	child_process_init(&process);
> > +
> > +	prepare_pager_args(&pager_process, pager);
> > +	pager_process.in = -1;
> > +	strvec_push(&pager_process.env, "GIT_PAGER_IN_USE");
> > +	if (start_command(&pager_process))
> > +		return;
> > +
> > +	pager_stdin = fdopen(pager_process.in, "w");
> > +	fputs(s, pager_stdin);
> > +	fflush(pager_stdin);
> > +
> > +	close(pager_process.in);
> > +	finish_command(&pager_process);
> > +}
> > diff --git a/pager.h b/pager.h
> > index b77433026d..dcccfa632b 100644
> > --- a/pager.h
> > +++ b/pager.h
> > @@ -11,6 +11,7 @@ void term_clear_line(void);
> >  int decimal_width(uintmax_t);
> >  int check_pager_config(const char *cmd);
> >  void prepare_pager_args(struct child_process *, const char *pager);
> > +void fputs_to_pager(const char* s);
> > 
> >  extern int pager_use_color;