=?UTF-8?B?QnJhbmtvIMSMaWJlag==?= <brane_at_xbc.nu> writes:
> Eric Gillespie wrote:
> > =?UTF-8?B?QnJhbmtvIMSMaWJlag==?= <brane_at_xbc.nu> writes:
> >
> >
> >> The isatty hack should be in APR; at least we should make an apr-like
> >> function to do that and submit it for inclusion in APR. This way ... is
> >> not nice.
> >>
> >
> > So, I'm pretty sure the only way APR is going to want this is if
> > I add a function to tell if an apr_file_t is a tty. Maybe that's
> > useful, but it seems ridiculous to me to create an apr_file_t for
> > no other reason than to ask if it's a tty, and then just throw it
> > away (and in some cases, turn around and create another one via
> > svn_stream_for_stdout.
> >
> > So, I'm reluctant to go that route, but I can do it if this is
> > going to block this change going in. Also, I can break this bit
> > out into svn_io_isatty and have it take a FILE * even though that
> > won't end up going back into apr, if you like.
> >
>
> You have a point. In that case, best leave all this with a TODO comment.
TODO added. Here's the updated patch and unchanged log message.
[[[
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; see svn_stream_for_pager for TODOs */
+#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,88 @@
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);
+
+ /* TODO: We should consider adding isatty to apr and using that. Of
+ * course, they're likely to want to use an apr_file_t, which means
+ * creating one here just for this test, then throwing it away, and
+ * maybe then creating a new one in svn_stream_for_stdout. Seems
+ * rather pointless for such a simple test.
+ */
+#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-12 02:41:37 CET