You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/02/06 01:16:47 UTC

svn commit: r1565011 - in /subversion/branches/automatic-pager/subversion: include/svn_cmdline.h libsvn_subr/cmdline.c svn/blame-cmd.c svn/cl.h svn/diff-cmd.c svn/help-cmd.c svn/svn.c svn/util.c

Author: stsp
Date: Thu Feb  6 00:16:47 2014
New Revision: 1565011

URL: http://svn.apache.org/r1565011
Log:
On the automatic-pager branch, rework the API that controls the pager.

This is a more recent work-in-progress state of my patch than committed
by breser. It's not final yet but can be tested and tweaked easily.

* subversion/include/svn_cmdline.h
  (svn_cmline_start_pager): Declare.

* subversion/libsvn_subr/cmdline.c
  (close_pager_baton): New.
  (close_pager): New helper function. This is a pool cleanup handler
   that is used to close the pager on process exit.
  (parse_pager_env): Parse the PAGER/SVN_PAGER env vars. Splits arguments
   but heeds single/double quoting and backslash escaping so that paths
   in the pager command line may contain whitespace.
   FIXME: This function currently lacks a docstring.
  (svn_cmdline_start_pager): The public API to enable use of a pager.
   Applies some option hacks for 'less' inspired by git, though I'm not
   sure yet if we'll keep them.

* subversion/svn/blame-cmd.c,
  subversion/svn/diff-cmd.c,
  subversion/svn/help-cmd.c:
  (svn_cl__blame, svn_cl__diff, svn_cl__help): Use simplified pager API.

* subversion/svn/svn.c

* subversion/svn/cl.h,
  subversion/svn/util.c:
  (svn_cl__start_pager, svn_cl__close_pager): Remove code added in r1564998
   superseded by this commit.

Modified:
    subversion/branches/automatic-pager/subversion/include/svn_cmdline.h
    subversion/branches/automatic-pager/subversion/libsvn_subr/cmdline.c
    subversion/branches/automatic-pager/subversion/svn/blame-cmd.c
    subversion/branches/automatic-pager/subversion/svn/cl.h
    subversion/branches/automatic-pager/subversion/svn/diff-cmd.c
    subversion/branches/automatic-pager/subversion/svn/help-cmd.c
    subversion/branches/automatic-pager/subversion/svn/svn.c
    subversion/branches/automatic-pager/subversion/svn/util.c

Modified: subversion/branches/automatic-pager/subversion/include/svn_cmdline.h
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/include/svn_cmdline.h?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/include/svn_cmdline.h (original)
+++ subversion/branches/automatic-pager/subversion/include/svn_cmdline.h Thu Feb  6 00:16:47 2014
@@ -369,6 +369,17 @@ svn_cmdline_setup_auth_baton(svn_auth_ba
                              void *cancel_baton,
                              apr_pool_t *pool);
 
+/* Launch an external pager program and redirect stdout to the pager.
+ * The pager is not started if the PAGER or SVN_PAGER environment
+ * variables are not set, or if standard output is not a terminal.
+ * The pager process is terminated from a cleanup handler of @a RESULT_POOL.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_cmdline_start_pager(apr_pool_t *result_pool,
+                        apr_pool_t *scratch_pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/branches/automatic-pager/subversion/libsvn_subr/cmdline.c
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/libsvn_subr/cmdline.c?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/libsvn_subr/cmdline.c (original)
+++ subversion/branches/automatic-pager/subversion/libsvn_subr/cmdline.c Thu Feb  6 00:16:47 2014
@@ -89,6 +89,214 @@ static svn_boolean_t shortcut_stdout_to_
 static svn_boolean_t shortcut_stderr_to_console = FALSE;
 #endif
 
+struct close_pager_baton {
+  apr_proc_t *pager_proc;
+  apr_pool_t *scratch_pool;
+};
+
+/* Close the pager with process handle PAGER_PROC.
+ * Must be called after all output has been written. */
+static apr_status_t
+close_pager(void *baton)
+{
+  struct close_pager_baton *b = baton;
+  apr_proc_t *pager_proc = b->pager_proc;
+  apr_pool_t *scratch_pool = b->scratch_pool;
+  apr_file_t *stdout_file;
+  apr_status_t apr_err;
+  svn_error_t *err;
+
+  apr_err = apr_file_open_stdout(&stdout_file, scratch_pool);
+  if (apr_err)
+    {
+      svn_handle_warning2(stderr,
+                          svn_error_wrap_apr(apr_err, "Can't open stdout"),
+                          "svn: ");
+      svn_error_clear(err);
+    }
+
+  err = svn_io_file_close(stdout_file, scratch_pool);
+  if (err)
+    {
+      svn_handle_warning2(stderr, err, "svn: ");
+      svn_error_clear(err);
+    }
+
+  err = svn_io_file_close(pager_proc->in, scratch_pool);
+  if (err)
+    {
+      svn_handle_warning2(stderr, err, "svn: ");
+      svn_error_clear(err);
+    }
+
+  err = svn_io_wait_for_cmd(pager_proc,
+                            apr_psprintf(scratch_pool, "%ld",
+                                         (long)pager_proc->pid),
+                            NULL, NULL, scratch_pool);
+  if (err)
+    {
+      svn_handle_warning2(stderr, err, "svn: ");
+      svn_error_clear(err);
+      svn_cmdline_fputs(_("svn: There was a problem with the pager. Please "
+                          "check whether the SVN_PAGER or PAGER environment "
+                          "variables are set correctly.\n"),
+                        stderr, scratch_pool);
+      svn_cmdline_fflush(stderr);
+    }
+
+  return SVN_NO_ERROR;
+}
+
+static apr_array_header_t *
+parse_pager_env(const char *pager_env, apr_pool_t *result_pool)
+{
+  apr_array_header_t *pager_cmd = apr_array_make(result_pool, 0,
+                                                 sizeof(const char *));
+  const char *p;
+  svn_boolean_t single_quote = FALSE;
+  svn_boolean_t double_quote = FALSE;
+#define quoted (single_quote || double_quote)
+  svn_stringbuf_t *next_arg = svn_stringbuf_create_empty(result_pool);
+
+  p = pager_env;
+  while (*p)
+    {
+      switch (*p)
+        {
+          case ' ':
+          case '\t':
+            if (quoted)
+              svn_stringbuf_appendbyte(next_arg, *p);
+            else
+              {
+                if (next_arg->len > 0)
+                  APR_ARRAY_PUSH(pager_cmd, const char *) =
+                    apr_pstrdup(result_pool, next_arg->data);
+                svn_stringbuf_setempty(next_arg);
+              }
+            break;
+
+          case '"':
+            if (single_quote)
+              svn_stringbuf_appendbyte(next_arg, *p);
+            else
+              double_quote = !double_quote;
+            break;
+
+          case '\'':
+            if (double_quote)
+              svn_stringbuf_appendbyte(next_arg, *p);
+            else
+              single_quote = !single_quote;
+            break;
+
+          case '\\':
+            if (quoted)
+              svn_stringbuf_appendbyte(next_arg, *p);
+            else if (*(p + 1))
+              {
+                svn_stringbuf_appendbyte(next_arg, *(p + 1));
+                p++;
+              }
+            break;
+
+          default:
+            svn_stringbuf_appendbyte(next_arg, *p);
+            break;
+        }
+      p++;
+    }
+
+  if (next_arg->len > 0)
+    APR_ARRAY_PUSH(pager_cmd, const char *) = apr_pstrdup(result_pool,
+                                                          next_arg->data);
+
+#undef quoted
+  return pager_cmd;
+}
+
+svn_error_t *
+svn_cmdline_start_pager(apr_pool_t *result_pool,
+                        apr_pool_t *scratch_pool)
+{
+  apr_proc_t *pager_proc;
+  apr_array_header_t *pager_cmd;
+  const char *pager;
+  const char **args;
+  apr_file_t *stdout_file;
+  apr_status_t apr_err;
+  int i;
+  struct close_pager_baton *close_pager_baton;
+
+  pager = getenv("SVN_PAGER");
+  if (pager == NULL)
+    pager = getenv("PAGER");
+  /* TODO get pager-cmd from config */
+  if (pager == NULL)
+    return SVN_NO_ERROR;
+
+#ifdef WIN32
+  if (_isatty(STDOUT_FILENO) == 0)
+    return SVN_NO_ERROR;
+#else
+  if (isatty(STDOUT_FILENO) == 0)
+    return SVN_NO_ERROR;
+#endif
+
+  pager_cmd = parse_pager_env(pager, scratch_pool);
+  if (pager_cmd->nelts <= 0)
+    return SVN_NO_ERROR;
+  args = apr_pcalloc(scratch_pool,
+                     (sizeof(const char *) * pager_cmd->nelts + 1));
+  for (i = 0; i < pager_cmd->nelts; i++)
+    {
+      const char *arg;
+      const char *arg_utf8;
+
+      arg = APR_ARRAY_IDX(pager_cmd, i, const char *);
+      SVN_ERR(svn_cmdline_cstring_to_utf8(&arg_utf8, arg, scratch_pool));
+      args[i] = arg_utf8;
+    }
+  args[i] = NULL;
+
+  args[0] = svn_dirent_canonicalize(args[0], scratch_pool);
+
+  /* Set default options for 'less' to -FX. */
+  if (strcmp(svn_dirent_basename(args[0], scratch_pool), "less") == 0
+      && args[1] == NULL)
+    {
+      const char *less_env = getenv("LESS");
+      svn_stringbuf_t *new_less_env;
+      
+      new_less_env = svn_stringbuf_create(less_env ? less_env : "", scratch_pool);
+      svn_stringbuf_appendcstr(new_less_env, "FX");
+      setenv("LESS", new_less_env->data, 1);
+    }
+
+  apr_err = apr_file_open_stdout(&stdout_file, scratch_pool);
+  if (apr_err)
+    return svn_error_wrap_apr(apr_err, "Can't open stdout");
+
+  pager_proc = apr_pcalloc(result_pool, sizeof(*pager_proc));
+  args[0] = svn_dirent_local_style(args[0], scratch_pool);
+  SVN_ERR(svn_io_start_cmd3(pager_proc, NULL, args[0], args, NULL, TRUE,
+                            TRUE, NULL,
+                            FALSE, NULL,
+                            FALSE, NULL,
+                            result_pool));
+
+  close_pager_baton = apr_pcalloc(result_pool, sizeof(*close_pager_baton));
+  close_pager_baton->pager_proc = pager_proc;
+  close_pager_baton->scratch_pool = result_pool;
+  apr_pool_cleanup_register(result_pool, close_pager_baton, close_pager, NULL);
+
+  SVN_ERR(svn_cmdline_fflush(stdout)); /* Flush existing output */
+  apr_err = apr_file_dup2(stdout_file, pager_proc->in, result_pool);
+  if (apr_err)
+    return svn_error_wrap_apr(apr_err, "Can't redirect stdout");
+
+  return SVN_NO_ERROR;
+}
 
 int
 svn_cmdline_init(const char *progname, FILE *error_stream)

Modified: subversion/branches/automatic-pager/subversion/svn/blame-cmd.c
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/svn/blame-cmd.c?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/svn/blame-cmd.c (original)
+++ subversion/branches/automatic-pager/subversion/svn/blame-cmd.c Thu Feb  6 00:16:47 2014
@@ -246,7 +246,6 @@ svn_cl__blame(apr_getopt_t *os,
   svn_boolean_t end_revision_unspecified = FALSE;
   svn_diff_file_options_t *diff_options = svn_diff_file_options_create(pool);
   svn_boolean_t seen_nonexistent_target = FALSE;
-  apr_proc_t *pager_proc = NULL;
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -318,7 +317,7 @@ svn_cl__blame(apr_getopt_t *os,
     }
 
   if (!opt_state->non_interactive && !opt_state->xml)
-    SVN_ERR(svn_cl__start_pager(&pager_proc, pool, pool));
+    SVN_ERR(svn_cmdline_start_pager(pool, pool));
 
   for (i = 0; i < targets->nelts; i++)
     {
@@ -413,9 +412,6 @@ svn_cl__blame(apr_getopt_t *os,
   if (opt_state->xml && ! opt_state->incremental)
     SVN_ERR(svn_cl__xml_print_footer("blame", pool));
 
-  if (pager_proc)
-    SVN_ERR(svn_cl__close_pager(pager_proc, pool));
-
   if (seen_nonexistent_target)
     return svn_error_create(
       SVN_ERR_ILLEGAL_TARGET, NULL,

Modified: subversion/branches/automatic-pager/subversion/svn/cl.h
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/svn/cl.h?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/svn/cl.h (original)
+++ subversion/branches/automatic-pager/subversion/svn/cl.h Thu Feb  6 00:16:47 2014
@@ -848,22 +848,6 @@ svn_cl__deprecated_merge_reintegrate(con
                                      svn_client_ctx_t *ctx,
                                      apr_pool_t *pool);
 
-
-/* Launch an external pager program and redirect stdout to the pager.
- *
- * If the pager was started, return a pointer to the process handle
- * for the pager in *PAGER_PROC. Else, set *PAGER_PROC to NULL. */
-svn_error_t *
-svn_cl__start_pager(apr_proc_t **pager_proc,
-                    apr_pool_t *result_pool,
-                    apr_pool_t *scratch_pool);
-
-/* Close the pager with process handle PAGER_PROC.
- * Must be called after all output has been written. */
-svn_error_t *
-svn_cl__close_pager(apr_proc_t *pager_proc,
-                    apr_pool_t *scratch_pool);
-
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/branches/automatic-pager/subversion/svn/diff-cmd.c
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/svn/diff-cmd.c?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/svn/diff-cmd.c (original)
+++ subversion/branches/automatic-pager/subversion/svn/diff-cmd.c Thu Feb  6 00:16:47 2014
@@ -188,7 +188,6 @@ svn_cl__diff(apr_getopt_t *os,
   struct summarize_baton_t summarize_baton;
   const svn_client_diff_summarize_func_t summarize_func =
     (opt_state->xml ? summarize_xml : summarize_regular);
-  apr_proc_t *pager_proc = NULL;
 
   if (opt_state->extensions)
     options = svn_cstring_split(opt_state->extensions, " \t\n\r", TRUE, pool);
@@ -357,7 +356,7 @@ svn_cl__diff(apr_getopt_t *os,
   svn_opt_push_implicit_dot_target(targets, pool);
 
   if (!opt_state->non_interactive && !opt_state->diff.summarize)
-    SVN_ERR(svn_cl__start_pager(&pager_proc, pool, pool));
+    SVN_ERR(svn_cmdline_start_pager(pool, pool));
 
   iterpool = svn_pool_create(pool);
 
@@ -492,8 +491,5 @@ svn_cl__diff(apr_getopt_t *os,
 
   svn_pool_destroy(iterpool);
 
-  if (pager_proc)
-    SVN_ERR(svn_cl__close_pager(pager_proc, pool));
-
   return SVN_NO_ERROR;
 }

Modified: subversion/branches/automatic-pager/subversion/svn/help-cmd.c
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/svn/help-cmd.c?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/svn/help-cmd.c (original)
+++ subversion/branches/automatic-pager/subversion/svn/help-cmd.c Thu Feb  6 00:16:47 2014
@@ -45,7 +45,6 @@ svn_cl__help(apr_getopt_t *os,
 {
   svn_cl__opt_state_t *opt_state = NULL;
   svn_stringbuf_t *version_footer = NULL;
-  apr_proc_t *pager_proc = NULL;
 
   char help_header[] =
   N_("usage: svn <subcommand> [options] [args]\n"
@@ -134,7 +133,7 @@ svn_cl__help(apr_getopt_t *os,
   SVN_ERR(svn_ra_print_modules(version_footer, pool));
 
   if (opt_state && !opt_state->non_interactive)
-    SVN_ERR(svn_cl__start_pager(&pager_proc, pool, pool));
+    SVN_ERR(svn_cmdline_start_pager(pool, pool));
 
   SVN_ERR(svn_opt_print_help4(os,
                               "svn",   /* ### erm, derive somehow? */
@@ -149,8 +148,5 @@ svn_cl__help(apr_getopt_t *os,
                               _(help_footer),
                               pool));
 
-  if (pager_proc)
-    SVN_ERR(svn_cl__close_pager(pager_proc, pool));
-
   return SVN_NO_ERROR;
 }

Modified: subversion/branches/automatic-pager/subversion/svn/svn.c
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/svn/svn.c?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/svn/svn.c (original)
+++ subversion/branches/automatic-pager/subversion/svn/svn.c Thu Feb  6 00:16:47 2014
@@ -2907,6 +2907,8 @@ sub_main(int *exit_code, int argc, const
     ctx->conflict_baton2 = b;
   }
 
+  SVN_ERR(svn_cmdline_start_pager(pool, pool));
+
   /* And now we finally run the subcommand. */
   err = (*subcommand->cmd_func)(os, &command_baton, pool);
   if (err)

Modified: subversion/branches/automatic-pager/subversion/svn/util.c
URL: http://svn.apache.org/viewvc/subversion/branches/automatic-pager/subversion/svn/util.c?rev=1565011&r1=1565010&r2=1565011&view=diff
==============================================================================
--- subversion/branches/automatic-pager/subversion/svn/util.c (original)
+++ subversion/branches/automatic-pager/subversion/svn/util.c Thu Feb  6 00:16:47 2014
@@ -33,15 +33,6 @@
 #include <ctype.h>
 #include <assert.h>
 
-#ifndef WIN32
-#include <unistd.h>
-#else
-#include <crtdbg.h>
-#include <io.h>
-#include <conio.h>
-#endif
-
-#include <apr.h>                /* for STDOUT_FILENO */
 #include <apr_env.h>
 #include <apr_errno.h>
 #include <apr_file_info.h>
@@ -1071,86 +1062,3 @@ svn_cl__propset_print_binary_mime_type_w
   return SVN_NO_ERROR;
 }
 
-svn_error_t *
-svn_cl__start_pager(apr_proc_t **pager_proc,
-                    apr_pool_t *result_pool,
-                    apr_pool_t *scratch_pool)
-{
-  apr_array_header_t *pager_cmd;
-  const char *pager;
-  const char **args;
-  apr_file_t *stdout_file;
-  apr_status_t apr_err;
-  int i;
-
-  *pager_proc = NULL;
-
-  pager = getenv("SVN_PAGER");
-  if (pager == NULL)
-    pager = getenv("PAGER");
-  /* TODO get pager-cmd from config */
-  if (pager == NULL)
-    return SVN_NO_ERROR;
-
-#ifdef WIN32
-  if (_isatty(STDOUT_FILENO) == 0)
-    return SVN_NO_ERROR;
-#else
-  if (isatty(STDOUT_FILENO) == 0)
-    return SVN_NO_ERROR;
-#endif
-
-  pager_cmd = svn_cstring_split(pager, " \t", TRUE, scratch_pool);
-  if (pager_cmd->nelts <= 0)
-    return SVN_NO_ERROR;
-  args = apr_pcalloc(scratch_pool,
-                     (sizeof(const char *) * pager_cmd->nelts + 1));
-  for (i = 0; i < pager_cmd->nelts; i++)
-    args[i] = APR_ARRAY_IDX(pager_cmd, i, const char *);
-  args[i] = NULL;
-
-  apr_err = apr_file_open_stdout(&stdout_file, scratch_pool);
-  if (apr_err)
-    return svn_error_wrap_apr(apr_err, "Can't open stdout");
-
-  *pager_proc = apr_pcalloc(result_pool, sizeof(**pager_proc));
-  SVN_ERR(svn_io_start_cmd3(*pager_proc, NULL, args[0], args, NULL, TRUE,
-                            TRUE, NULL,
-                            FALSE, NULL,
-                            FALSE, NULL,
-                            result_pool));
-
-  apr_err = apr_file_dup2(stdout_file, (*pager_proc)->in, result_pool);
-  if (apr_err)
-    return svn_error_wrap_apr(apr_err, "Can't redirect stdout");
-
-  return SVN_NO_ERROR;
-}
-
-svn_error_t *
-svn_cl__close_pager(apr_proc_t *pager_proc,
-                    apr_pool_t *scratch_pool)
-{
-  apr_file_t *stdout_file;
-  apr_status_t apr_err;
-  svn_error_t *err;
-
-  apr_err = apr_file_open_stdout(&stdout_file, scratch_pool);
-  if (apr_err)
-    return svn_error_wrap_apr(apr_err, "Can't open stdout");
-  SVN_ERR(svn_io_file_close(stdout_file, scratch_pool));
-  SVN_ERR(svn_io_file_close(pager_proc->in, scratch_pool));
-  err = svn_io_wait_for_cmd(pager_proc,
-                            apr_psprintf(scratch_pool, "%ld",
-                                         (long)pager_proc->pid),
-                            NULL, NULL, scratch_pool);
-  if (err && err->apr_err == SVN_ERR_EXTERNAL_PROGRAM)
-    {
-      svn_handle_warning2(stderr, err, "svn: ");
-      svn_error_clear(err);
-    }
-  else
-    SVN_ERR(err);
-
-  return SVN_NO_ERROR;
-}