[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Don't blow out scrollback in conflict handler prompt diff

From: Eric Gillespie <epg_at_google.com>
Date: Mon, 11 Feb 2008 11:58:13 -0800

Many users have very little scrollback in their terminal
emulators, and keep complaining to me about the diff option in
our conflict handling uselessly blowing past that, ignoring their
PAGER settings. This patch implements a new pager stream and
uses it for the diff.

I've tested on Ubuntu Dapper and Windows XP, both on i386. On
both platforms, the tty detection works, and the pager is not
invoked when stdout is redirected. I don't know how to get a
pager on Windows, though. 'more' seems to work at the cmd.exe
prompt but not via start_cmd:

Conflict discovered in 'subversion/libsvn_fs_fs/tree.c'.
Select: (p)ostpone, (d)iff, (e)dit, (h)elp for more options : d
svn: Can't start process 'more': The system cannot find the file specified.
--- C:\DOCUME~1\epg\LOCALS~1\Temp/tmp Fri Feb 8 18:01:08 2008
+++ subversion/libsvn_fs_fs/.svn/tmp/tempfile.114.tmp Fri Feb 8 18:01:08 2008

I haven't made things *worse* on Windows, however, so that's no
reason to reject this. That said, it would be nice if someone
could fix that part of things.

[[[
Run diff output through the user's pager, if any, rather than
dumping it all to the standard output.

* subversion/libsvn_subr/stream.c
  (pager_baton): Add baton for new stream.
  (write_handler_pager): Add write handler that just passes data along
    to the apr file stream in the baton.
  (close_handler_pager): Add close handler that closes the apr file
    stream in the baton and svn_io_wait_for_cmd()s for the apr_proc_t
    in the baton.
  (svn_stream_for_pager): Add function that creates a stream for the
    user's pager if configured in the PAGER environment variable and
    if the standard output is a tty, else a generic stream connected
    to stdout.

* subversion/include/svn_io.h
  (svn_stream_for_pager): Add prototype.

* subversion/svn/conflict-callbacks.c
  (show_diff): Try to use svn_stream_for_pager before trying
    svn_stream_for_stdout. Print any errors other than EPIPE from
    pager handling to stderr but don't return them.
]]]

Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 29243)
+++ subversion/include/svn_io.h (working copy)
@@ -621,6 +621,18 @@
  */
 svn_error_t *svn_stream_for_stdout(svn_stream_t **out, apr_pool_t *pool);
 
+/** Set @a *out to a generic stream connected to the user's pager if
+ * configured in the PAGER environment variable and if the standard
+ * output is a tty, else return a generic stream connected to stdout.
+ * Allocate the new stream in @a pool. The stream and its underlying
+ * APR handle and child process will be closed by svn_stream_close or
+ * when @a pool is cleared or destroyed.
+ *
+ * @since New in 1.5.
+ */
+svn_error_t *
+svn_stream_for_pager(svn_stream_t **out, apr_pool_t *pool);
+
 /** Return a generic stream connected to stringbuf @a str. Allocate the
  * stream in @a pool.
  */
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 29243)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -21,8 +21,18 @@
 #include <assert.h>
 #include <stdio.h>
 
+/* for isatty */
+#if HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#ifdef WIN32
+#include <io.h>
+#endif
+
 #include <apr.h>
+#include <apr_env.h>
 #include <apr_pools.h>
+#include <apr_portable.h>
 #include <apr_strings.h>
 #include <apr_file_io.h>
 #include <apr_errno.h>
@@ -883,3 +893,82 @@
 
   return SVN_NO_ERROR;
 }
+
+
+struct pager_baton {
+ apr_proc_t cmd_proc;
+ char *cmd;
+ svn_stream_t *write_stdin;
+ apr_pool_t *pool;
+};
+
+static svn_error_t *
+write_handler_pager(void *baton, const char *data, apr_size_t *len)
+{
+ struct pager_baton *pb = baton;
+ return svn_stream_write(pb->write_stdin, data, len);
+}
+
+static svn_error_t *
+close_handler_pager(void *baton)
+{
+ struct pager_baton *pb = baton;
+ SVN_ERR(svn_stream_close(pb->write_stdin));
+ return svn_io_wait_for_cmd(&pb->cmd_proc, pb->cmd, NULL, NULL,
+ pb->pool);
+}
+
+svn_error_t *
+svn_stream_for_pager(svn_stream_t **out, apr_pool_t *pool)
+{
+ struct pager_baton *pb = apr_palloc(pool, sizeof(*pb));
+ svn_boolean_t tty;
+ const char *argv[2];
+ apr_status_t apr_err;
+ apr_file_t *read_stdin, *write_stdin;
+
+ if (apr_env_get(&pb->cmd, "PAGER", pool) != APR_SUCCESS)
+ return svn_stream_for_stdout(out, pool);
+
+#ifdef WIN32
+ tty = _isatty(_fileno(stdout));
+#else
+ tty = isatty(fileno(stdout));
+#endif
+ if (!tty)
+ return svn_stream_for_stdout(out, pool);
+
+ /* Assemble argv for pager. */
+ argv[0] = pb->cmd;
+ argv[1] = NULL;
+ /* Assemble pipe for pager's standard input. */
+ apr_err = apr_file_pipe_create(&read_stdin, &write_stdin, pool);
+ if (apr_err)
+ return svn_error_wrap_apr
+ (apr_err, _("Can't create pipe for pager '%s'"), pb->cmd);
+ /* Tell apr to close the write side of the pipe in the child. */
+ apr_err = apr_file_inherit_unset(write_stdin);
+ if (apr_err)
+ return svn_error_wrap_apr
+ (apr_err, _("Can't make pipe write handle non-inherited for pager '%s'"),
+ pb->cmd);
+ /* Run the pager with standard input coming from this stream and
+ standard output and error alone (typically a tty). */
+ SVN_ERR(svn_io_start_cmd(&pb->cmd_proc, ".", pb->cmd, argv,
+ TRUE, /* inherit environment */
+ read_stdin, /* infile */
+ NULL, /* outfile */
+ NULL, /* errfile */
+ pool));
+ SVN_ERR(svn_io_file_close(read_stdin, pool));
+
+ pb->write_stdin = svn_stream_from_aprfile2(write_stdin,
+ FALSE, /* disown */
+ pool);
+ pb->pool = pool;
+ *out = svn_stream_create(pb, pool);
+ svn_stream_set_write(*out, write_handler_pager);
+ svn_stream_set_close(*out, close_handler_pager);
+
+ return SVN_NO_ERROR;
+}
Index: subversion/svn/conflict-callbacks.c
===================================================================
--- subversion/svn/conflict-callbacks.c (revision 29243)
+++ subversion/svn/conflict-callbacks.c (working copy)
@@ -89,6 +89,7 @@
   svn_diff_t *diff;
   svn_stream_t *output;
   svn_diff_file_options_t *options;
+ svn_error_t *err;
 
   if (desc->merged_file && desc->base_file)
     {
@@ -106,15 +107,35 @@
 
   options = svn_diff_file_options_create(pool);
   options->ignore_eol_style = TRUE;
- SVN_ERR(svn_stream_for_stdout(&output, pool));
   SVN_ERR(svn_diff_file_diff_2(&diff, path1, path2,
                                options, pool));
- SVN_ERR(svn_diff_file_output_unified2(output, diff,
- path1, path2,
- NULL, NULL,
- APR_LOCALE_CHARSET,
- pool));
 
+ /* Get output stream for pager or standard output. */
+ err = svn_stream_for_pager(&output, pool);
+ if (err != SVN_NO_ERROR)
+ {
+ svn_handle_error2(err, stderr, FALSE, "svn: ");
+ svn_error_clear(err);
+ SVN_ERR(svn_stream_for_stdout(&output, pool));
+ }
+
+ err = svn_diff_file_output_unified2(output, diff,
+ path1, path2,
+ NULL, NULL,
+ APR_LOCALE_CHARSET,
+ pool);
+
+ /* Ignore broken pipes from the pager, which just means it exited
+ before consuming all input. */
+ if (err != SVN_NO_ERROR)
+ if (!APR_STATUS_IS_EPIPE(err->apr_err))
+ svn_handle_error2(err, stderr, FALSE, "svn: ");
+ svn_error_clear(err);
+ err = svn_stream_close(output);
+ if (err != SVN_NO_ERROR)
+ svn_handle_error2(err, stderr, FALSE, "svn: ");
+ svn_error_clear(err);
+
   *performed_edit = TRUE;
 
   return SVN_NO_ERROR;

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-11 20:58:30 CET

This is an archived mail posted to the Subversion Dev mailing list.